From 5a41e6faa8cc9974aeee70336051baad6d3dd046 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 4 Oct 2022 14:40:59 -0700 Subject: [PATCH 1/3] Fix deprecated API bug --- src/client/deprecatedProposedApi.ts | 7 +++++++ src/client/deprecatedProposedApiTypes.ts | 9 +++++++++ src/client/proposedApi.ts | 14 +++++--------- src/test/proposedApi.unit.test.ts | 16 ++++++---------- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/client/deprecatedProposedApi.ts b/src/client/deprecatedProposedApi.ts index 459f2b1bf529..e7e854dc0661 100644 --- a/src/client/deprecatedProposedApi.ts +++ b/src/client/deprecatedProposedApi.ts @@ -90,6 +90,13 @@ export function buildDeprecatedProposedApi( const env = await interpreterService.getActiveInterpreter(resource); return env ? { execCommand: [env.path] } : { execCommand: undefined }; }, + async getActiveEnvironmentPath(resource?: Resource) { + const env = await interpreterService.getActiveInterpreter(resource); + if (!env) { + return undefined; + } + return getEnvPath(env.path, env.envPath); + }, async getEnvironmentDetails( path: string, options?: EnvironmentDetailsOptions, diff --git a/src/client/deprecatedProposedApiTypes.ts b/src/client/deprecatedProposedApiTypes.ts index 864185483504..f2a2cbe040af 100644 --- a/src/client/deprecatedProposedApiTypes.ts +++ b/src/client/deprecatedProposedApiTypes.ts @@ -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; /** * 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 diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index 46518755116e..75afdac6469f 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -151,18 +151,19 @@ export function buildProposedApi( ); /** - * @deprecated Will be removed soon. Use {@link ProposedExtensionAPI.environments} 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 = {}; // Errors out only in case of testing. // Also, these APIs no longer supported, no need to log error. } const proposed: ProposedExtensionAPI = { + ...deprecatedProposedApi, environments: { getActiveEnvironmentPath(resource?: Resource) { sendApiTelemetry('getActiveEnvironmentPath'); @@ -172,10 +173,6 @@ export function buildProposedApi( return { id, path, - /** - * @deprecated Only provided for backwards compatibility and will soon be removed. - */ - pathType: 'interpreterPath', }; }, updateActiveEnvironmentPath( @@ -228,7 +225,6 @@ export function buildProposedApi( return onEnvironmentsChanged.event; }, }, - ...deprecatedEnvironmentsApi, }; return proposed; } diff --git a/src/test/proposedApi.unit.test.ts b/src/test/proposedApi.unit.test.ts index 8058c8ada4e3..a4358c0830fc 100644 --- a/src/test/proposedApi.unit.test.ts +++ b/src/test/proposedApi.unit.test.ts @@ -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'; @@ -92,11 +91,10 @@ suite('Proposed Extension API', () => { .setup((c) => c.getSettings(undefined)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); const actual = proposed.environments.getActiveEnvironmentPath(); - assert.deepEqual(actual, ({ + assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath, - pathType: 'interpreterPath', - } as unknown) as EnvironmentPath); + }); }); test('getActiveEnvironmentPath: default python', () => { @@ -105,11 +103,10 @@ suite('Proposed Extension API', () => { .setup((c) => c.getSettings(undefined)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); const actual = proposed.environments.getActiveEnvironmentPath(); - assert.deepEqual(actual, ({ + assert.deepEqual(actual, { id: 'DEFAULT_PYTHON', path: pythonPath, - pathType: 'interpreterPath', - } as unknown) as EnvironmentPath); + }); }); test('getActiveEnvironmentPath: With resource', () => { @@ -119,11 +116,10 @@ suite('Proposed Extension API', () => { .setup((c) => c.getSettings(resource)) .returns(() => (({ pythonPath } as unknown) as IPythonSettings)); const actual = proposed.environments.getActiveEnvironmentPath(resource); - assert.deepEqual(actual, ({ + assert.deepEqual(actual, { id: normCasePath(pythonPath), path: pythonPath, - pathType: 'interpreterPath', - } as unknown) as EnvironmentPath); + }); }); test('resolveEnvironment: invalid environment (when passed as string)', async () => { From f20485b77ba788d7846c623075d70f7656bfab84 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 4 Oct 2022 14:43:29 -0700 Subject: [PATCH 2/3] Rename API telemetry for deprecated APIs --- src/client/deprecatedProposedApi.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/client/deprecatedProposedApi.ts b/src/client/deprecatedProposedApi.ts index e7e854dc0661..e63670e4bf1b 100644 --- a/src/client/deprecatedProposedApi.ts +++ b/src/client/deprecatedProposedApi.ts @@ -86,11 +86,12 @@ 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; @@ -101,7 +102,7 @@ export function buildDeprecatedProposedApi( path: string, options?: EnvironmentDetailsOptions, ): Promise { - sendApiTelemetry('getEnvironmentDetails'); + sendApiTelemetry('deprecated.getEnvironmentDetails'); let env: PythonEnvInfo | undefined; if (options?.useCache) { env = discoveryApi.getEnvs().find((v) => isEnvSame(path, v)); @@ -125,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 { - 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 | 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; }, }, From ab7bd147cdfe04f6908146b5c75a91f11e890c91 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 4 Oct 2022 14:59:49 -0700 Subject: [PATCH 3/3] Explicitly type it to be safe --- src/client/proposedApi.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client/proposedApi.ts b/src/client/proposedApi.ts index 75afdac6469f..288d75a0f2b5 100644 --- a/src/client/proposedApi.ts +++ b/src/client/proposedApi.ts @@ -32,6 +32,7 @@ import { reportActiveInterpreterChangedDeprecated, reportInterpretersChanged, } from './deprecatedProposedApi'; +import { DeprecatedProposedAPI } from './deprecatedProposedApiTypes'; type ActiveEnvironmentChangeEvent = { resource: WorkspaceFolder | undefined; @@ -157,12 +158,12 @@ export function buildProposedApi( try { deprecatedProposedApi = { ...buildDeprecatedProposedApi(discoveryApi, serviceContainer) }; } catch (ex) { - deprecatedProposedApi = {}; + deprecatedProposedApi = {} as DeprecatedProposedAPI; // Errors out only in case of testing. // Also, these APIs no longer supported, no need to log error. } - const proposed: ProposedExtensionAPI = { + const proposed: ProposedExtensionAPI & DeprecatedProposedAPI = { ...deprecatedProposedApi, environments: { getActiveEnvironmentPath(resource?: Resource) {