Skip to content

Commit

Permalink
Cherry pick changes related to environment APIs (#19950)
Browse files Browse the repository at this point in the history
- Rename proposed api environment namespace to environments (#19938)
- Fix deprecated API bug (#19944)
  • Loading branch information
Kartik Raj authored Oct 6, 2022
1 parent e28b10d commit 1c33fd9
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 57 deletions.
28 changes: 18 additions & 10 deletions src/client/deprecatedProposedApi.ts
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
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
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
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
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

0 comments on commit 1c33fd9

Please sign in to comment.