Skip to content

Cherry pick changes related to environment APIs #19950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karrtikr merged 2 commits into from
Oct 6, 2022
Merged

Cherry pick changes related to environment APIs #19950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8307b41
Rename proposed api `environment` namespace to `environments` (#19938)
Oct 4, 2022
9914141
Fix deprecated API bug (#19944)
Oct 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions src/client/deprecatedProposedApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,23 @@ export function buildDeprecatedProposedApi(
const proposed: DeprecatedProposedAPI = {
environment: {
async getExecutionDetails(resource?: Resource) {
sendApiTelemetry('getExecutionDetails');
sendApiTelemetry('deprecated.getExecutionDetails');
const env = await interpreterService.getActiveInterpreter(resource);
return env ? { execCommand: [env.path] } : { execCommand: undefined };
},
async getActiveEnvironmentPath(resource?: Resource) {
sendApiTelemetry('deprecated.getActiveEnvironmentPath');
const env = await interpreterService.getActiveInterpreter(resource);
if (!env) {
return undefined;
}
return getEnvPath(env.path, env.envPath);
},
async getEnvironmentDetails(
path: string,
options?: EnvironmentDetailsOptions,
): Promise<EnvironmentDetails | undefined> {
sendApiTelemetry('getEnvironmentDetails');
sendApiTelemetry('deprecated.getEnvironmentDetails');
let env: PythonEnvInfo | undefined;
if (options?.useCache) {
env = discoveryApi.getEnvs().find((v) => isEnvSame(path, v));
Expand All @@ -118,38 +126,38 @@ export function buildDeprecatedProposedApi(
};
},
getEnvironmentPaths() {
sendApiTelemetry('getEnvironmentPaths');
sendApiTelemetry('deprecated.getEnvironmentPaths');
const paths = discoveryApi.getEnvs().map((e) => getEnvPath(e.executable.filename, e.location));
return Promise.resolve(paths);
},
setActiveEnvironment(path: string, resource?: Resource): Promise<void> {
sendApiTelemetry('setActiveEnvironment');
sendApiTelemetry('deprecated.setActiveEnvironment');
return interpreterPathService.update(resource, ConfigurationTarget.WorkspaceFolder, path);
},
async refreshEnvironment() {
sendApiTelemetry('refreshEnvironment');
sendApiTelemetry('deprecated.refreshEnvironment');
await discoveryApi.triggerRefresh();
const paths = discoveryApi.getEnvs().map((e) => getEnvPath(e.executable.filename, e.location));
return Promise.resolve(paths);
},
getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise<void> | undefined {
sendApiTelemetry('getRefreshPromise');
sendApiTelemetry('deprecated.getRefreshPromise');
return discoveryApi.getRefreshPromise(options);
},
get onDidChangeExecutionDetails() {
sendApiTelemetry('onDidChangeExecutionDetails', false);
sendApiTelemetry('deprecated.onDidChangeExecutionDetails', false);
return interpreterService.onDidChangeInterpreterConfiguration;
},
get onDidEnvironmentsChanged() {
sendApiTelemetry('onDidEnvironmentsChanged', false);
sendApiTelemetry('deprecated.onDidEnvironmentsChanged', false);
return onDidInterpretersChangedEvent.event;
},
get onDidActiveEnvironmentChanged() {
sendApiTelemetry('onDidActiveEnvironmentChanged', false);
sendApiTelemetry('deprecated.onDidActiveEnvironmentChanged', false);
return onDidActiveInterpreterChangedEvent.event;
},
get onRefreshProgress() {
sendApiTelemetry('onRefreshProgress', false);
sendApiTelemetry('deprecated.onRefreshProgress', false);
return discoveryApi.onProgress;
},
},
Expand Down
13 changes: 11 additions & 2 deletions src/client/deprecatedProposedApiTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export interface ActiveEnvironmentChangedParams {
*/
export interface DeprecatedProposedAPI {
/**
* @deprecated Use {@link ProposedExtensionAPI.environment} instead. This will soon be removed.
* @deprecated Use {@link ProposedExtensionAPI.environments} instead. This will soon be removed.
*/
environment: {
/**
Expand Down Expand Up @@ -74,6 +74,15 @@ export interface DeprecatedProposedAPI {
*/
execCommand: string[] | undefined;
}>;
/**
* Returns the path to the python binary selected by the user or as in the settings.
* This is just the path to the python binary, this does not provide activation or any
* other activation command. The `resource` if provided will be used to determine the
* python binary in a multi-root scenario. If resource is `undefined` then the API
* returns what ever is set for the workspace.
* @param resource : Uri of a file or workspace
*/
getActiveEnvironmentPath(resource?: Resource): Promise<EnvPathType | undefined>;
/**
* Returns details for the given interpreter. Details such as absolute interpreter path,
* version, type (conda, pyenv, etc). Metadata such as `sysPrefix` can be found under
Expand Down Expand Up @@ -131,7 +140,7 @@ export interface DeprecatedProposedAPI {
*/
onDidEnvironmentsChanged: Event<EnvironmentsChangedParams[]>;
/**
* @deprecated Use {@link ProposedExtensionAPI.environment} `onDidChangeActiveEnvironmentPath` instead. This will soon be removed.
* @deprecated Use {@link ProposedExtensionAPI.environments} `onDidChangeActiveEnvironmentPath` instead. This will soon be removed.
*/
onDidActiveEnvironmentChanged: Event<ActiveEnvironmentChangedParams>;
};
Expand Down
23 changes: 10 additions & 13 deletions src/client/proposedApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
reportActiveInterpreterChangedDeprecated,
reportInterpretersChanged,
} from './deprecatedProposedApi';
import { DeprecatedProposedAPI } from './deprecatedProposedApiTypes';

type ActiveEnvironmentChangeEvent = {
resource: WorkspaceFolder | undefined;
Expand Down Expand Up @@ -151,19 +152,20 @@ export function buildProposedApi(
);

/**
* @deprecated Will be removed soon. Use {@link ProposedExtensionAPI.environment} instead.
* @deprecated Will be removed soon. Use {@link ProposedExtensionAPI} instead.
*/
let deprecatedEnvironmentsApi;
let deprecatedProposedApi;
try {
deprecatedEnvironmentsApi = { ...buildDeprecatedProposedApi(discoveryApi, serviceContainer).environment };
deprecatedProposedApi = { ...buildDeprecatedProposedApi(discoveryApi, serviceContainer) };
} catch (ex) {
deprecatedEnvironmentsApi = {};
deprecatedProposedApi = {} as DeprecatedProposedAPI;
// Errors out only in case of testing.
// Also, these APIs no longer supported, no need to log error.
}

const proposed: ProposedExtensionAPI = {
environment: {
const proposed: ProposedExtensionAPI & DeprecatedProposedAPI = {
...deprecatedProposedApi,
environments: {
getActiveEnvironmentPath(resource?: Resource) {
sendApiTelemetry('getActiveEnvironmentPath');
resource = resource && 'uri' in resource ? resource.uri : resource;
Expand All @@ -172,10 +174,6 @@ export function buildProposedApi(
return {
id,
path,
/**
* @deprecated Only provided for backwards compatibility and will soon be removed.
*/
pathType: 'interpreterPath',
};
},
updateActiveEnvironmentPath(
Expand Down Expand Up @@ -213,8 +211,8 @@ export function buildProposedApi(
sendApiTelemetry('resolveEnvironment');
return resolveEnvironment(path, discoveryApi);
},
get all(): Environment[] {
sendApiTelemetry('all');
get known(): Environment[] {
sendApiTelemetry('known');
return discoveryApi.getEnvs().map((e) => convertEnvInfoAndGetReference(e));
},
async refreshEnvironments(options?: RefreshOptions) {
Expand All @@ -227,7 +225,6 @@ export function buildProposedApi(
sendApiTelemetry('onDidChangeEnvironments');
return onEnvironmentsChanged.event;
},
...deprecatedEnvironmentsApi,
},
};
return proposed;
Expand Down
13 changes: 7 additions & 6 deletions src/client/proposedApiTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { CancellationToken, Event, Uri, WorkspaceFolder } from 'vscode';
// https://github.com/microsoft/vscode-python/wiki/Proposed-Environment-APIs

export interface ProposedExtensionAPI {
readonly environment: {
readonly environments: {
/**
* Returns the environment configured by user in settings.
* Returns the environment configured by user in settings. Note that this can be an invalid environment, use
* {@link resolveEnvironment} to get full details.
* @param resource : Uri of a file or workspace folder. This is used to determine the env in a multi-root
* scenario. If `undefined`, then the API returns what ever is set for the workspace.
*/
Expand All @@ -29,10 +30,10 @@ export interface ProposedExtensionAPI {
*/
readonly onDidChangeActiveEnvironmentPath: Event<ActiveEnvironmentPathChangeEvent>;
/**
* Carries environments found by the extension at the time of fetching the property. Note this may not
* Carries environments known to the extension at the time of fetching the property. Note this may not
* contain all environments in the system as a refresh might be going on.
*/
readonly all: readonly Environment[];
readonly known: readonly Environment[];
/**
* This event is triggered when the known environment list changes, like when a environment
* is found, existing environment is removed, or some details changed on an environment.
Expand All @@ -53,7 +54,7 @@ export interface ProposedExtensionAPI {
/**
* Returns details for the given environment, or `undefined` if the env is invalid.
* @param environment : Full path to environment folder or python executable for the environment. Can also pass
* the environment id or the environment itself.
* the environment itself.
*/
resolveEnvironment(
environment: Environment | EnvironmentPath | string,
Expand All @@ -63,7 +64,7 @@ export interface ProposedExtensionAPI {

export type RefreshOptions = {
/**
* Force trigger a refresh regardless of whether a refresh was already triggered. Note this can be expensive so
* When `true`, force trigger a refresh regardless of whether a refresh was already triggered. Note this can be expensive so
* it's best to only use it if user manually triggers a refresh.
*/
forceRefresh?: boolean;
Expand Down
48 changes: 22 additions & 26 deletions src/test/proposedApi.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import { PythonEnvCollectionChangedEvent } from '../client/pythonEnvironments/ba
import { normCasePath } from '../client/common/platform/fs-paths';
import {
ActiveEnvironmentPathChangeEvent,
EnvironmentPath,
EnvironmentsChangeEvent,
ProposedExtensionAPI,
} from '../client/proposedApiTypes';
Expand Down Expand Up @@ -76,7 +75,7 @@ suite('Proposed Extension API', () => {

test('Provide an event to track when active environment details change', async () => {
const events: ActiveEnvironmentPathChangeEvent[] = [];
proposed.environment.onDidChangeActiveEnvironmentPath((e) => {
proposed.environments.onDidChangeActiveEnvironmentPath((e) => {
events.push(e);
});
reportActiveInterpreterChanged({ path: 'path/to/environment', resource: undefined });
Expand All @@ -91,25 +90,23 @@ suite('Proposed Extension API', () => {
configService
.setup((c) => c.getSettings(undefined))
.returns(() => (({ pythonPath } as unknown) as IPythonSettings));
const actual = proposed.environment.getActiveEnvironmentPath();
assert.deepEqual(actual, ({
const actual = proposed.environments.getActiveEnvironmentPath();
assert.deepEqual(actual, {
id: normCasePath(pythonPath),
path: pythonPath,
pathType: 'interpreterPath',
} as unknown) as EnvironmentPath);
});
});

test('getActiveEnvironmentPath: default python', () => {
const pythonPath = 'python';
configService
.setup((c) => c.getSettings(undefined))
.returns(() => (({ pythonPath } as unknown) as IPythonSettings));
const actual = proposed.environment.getActiveEnvironmentPath();
assert.deepEqual(actual, ({
const actual = proposed.environments.getActiveEnvironmentPath();
assert.deepEqual(actual, {
id: 'DEFAULT_PYTHON',
path: pythonPath,
pathType: 'interpreterPath',
} as unknown) as EnvironmentPath);
});
});

test('getActiveEnvironmentPath: With resource', () => {
Expand All @@ -118,19 +115,18 @@ suite('Proposed Extension API', () => {
configService
.setup((c) => c.getSettings(resource))
.returns(() => (({ pythonPath } as unknown) as IPythonSettings));
const actual = proposed.environment.getActiveEnvironmentPath(resource);
assert.deepEqual(actual, ({
const actual = proposed.environments.getActiveEnvironmentPath(resource);
assert.deepEqual(actual, {
id: normCasePath(pythonPath),
path: pythonPath,
pathType: 'interpreterPath',
} as unknown) as EnvironmentPath);
});
});

test('resolveEnvironment: invalid environment (when passed as string)', async () => {
const pythonPath = 'this/is/a/test/path';
discoverAPI.setup((p) => p.resolveEnv(pythonPath)).returns(() => Promise.resolve(undefined));

const actual = await proposed.environment.resolveEnvironment(pythonPath);
const actual = await proposed.environments.resolveEnvironment(pythonPath);
expect(actual).to.be.equal(undefined);
});

Expand All @@ -150,7 +146,7 @@ suite('Proposed Extension API', () => {
});
discoverAPI.setup((p) => p.resolveEnv(pythonPath)).returns(() => Promise.resolve(env));

const actual = await proposed.environment.resolveEnvironment(pythonPath);
const actual = await proposed.environments.resolveEnvironment(pythonPath);
assert.deepEqual((actual as EnvironmentReference).internal, convertCompleteEnvInfo(env));
});

Expand All @@ -176,13 +172,13 @@ suite('Proposed Extension API', () => {
});
discoverAPI.setup((p) => p.resolveEnv(pythonPath)).returns(() => Promise.resolve(env));

const actual = await proposed.environment.resolveEnvironment(convertCompleteEnvInfo(partialEnv));
const actual = await proposed.environments.resolveEnvironment(convertCompleteEnvInfo(partialEnv));
assert.deepEqual((actual as EnvironmentReference).internal, convertCompleteEnvInfo(env));
});

test('environments: no pythons found', () => {
discoverAPI.setup((d) => d.getEnvs()).returns(() => []);
const actual = proposed.environment.all;
const actual = proposed.environments.known;
expect(actual).to.be.deep.equal([]);
});

Expand Down Expand Up @@ -232,7 +228,7 @@ suite('Proposed Extension API', () => {
},
];
discoverAPI.setup((d) => d.getEnvs()).returns(() => envs);
const actual = proposed.environment.all;
const actual = proposed.environments.known;
const actualEnvs = actual?.map((a) => (a as EnvironmentReference).internal);
assert.deepEqual(
actualEnvs?.sort((a, b) => a.id.localeCompare(b.id)),
Expand All @@ -244,7 +240,7 @@ suite('Proposed Extension API', () => {
let events: EnvironmentsChangeEvent[] = [];
let eventValues: EnvironmentsChangeEvent[] = [];
let expectedEvents: EnvironmentsChangeEvent[] = [];
proposed.environment.onDidChangeEnvironments((e) => {
proposed.environments.onDidChangeEnvironments((e) => {
events.push(e);
});
const envs = [
Expand Down Expand Up @@ -336,7 +332,7 @@ suite('Proposed Extension API', () => {
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path');
await proposed.environments.updateActiveEnvironmentPath('this/is/a/test/python/path');

interpreterPathService.verifyAll();
});
Expand All @@ -347,7 +343,7 @@ suite('Proposed Extension API', () => {
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.updateActiveEnvironmentPath({
await proposed.environments.updateActiveEnvironmentPath({
id: normCasePath('this/is/a/test/python/path'),
path: 'this/is/a/test/python/path',
});
Expand All @@ -362,7 +358,7 @@ suite('Proposed Extension API', () => {
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path', uri);
await proposed.environments.updateActiveEnvironmentPath('this/is/a/test/python/path', uri);

interpreterPathService.verifyAll();
});
Expand All @@ -379,7 +375,7 @@ suite('Proposed Extension API', () => {
index: 0,
};

await proposed.environment.updateActiveEnvironmentPath('this/is/a/test/python/path', workspace);
await proposed.environments.updateActiveEnvironmentPath('this/is/a/test/python/path', workspace);

interpreterPathService.verifyAll();
});
Expand All @@ -390,7 +386,7 @@ suite('Proposed Extension API', () => {
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.refreshEnvironments();
await proposed.environments.refreshEnvironments();

discoverAPI.verifyAll();
});
Expand All @@ -401,7 +397,7 @@ suite('Proposed Extension API', () => {
.returns(() => Promise.resolve())
.verifiable(typemoq.Times.once());

await proposed.environment.refreshEnvironments({ forceRefresh: true });
await proposed.environments.refreshEnvironments({ forceRefresh: true });

discoverAPI.verifyAll();
});
Expand Down