Skip to content

Commit c836cda

Browse files
Kartik Rajericsnowcurrently
andauthored
Do not call resolveEnv in component adapter where it's too expensive. (#14923)
* Do not call resolveEnv in component adapter where it's not needed * Fix imports * Added the OS check * Apply suggestions from code review Co-authored-by: Eric Snow <[email protected]> * Code reviews Co-authored-by: Eric Snow <[email protected]>
1 parent bc6c929 commit c836cda

File tree

2 files changed

+43
-24
lines changed

2 files changed

+43
-24
lines changed
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { getOSType, OSType } from '../../../../common/utils/platform';
5+
6+
// tslint:disable-next-line:no-suspicious-comment
7+
// TODO: Add tests for 'isMacDefaultPythonPath' when working on the locator
8+
9+
/**
10+
* Decide if the given Python executable looks like the MacOS default Python.
11+
*/
12+
export function isMacDefaultPythonPath(pythonPath: string): boolean {
13+
if (getOSType() !== OSType.OSX) {
14+
return false;
15+
}
16+
return pythonPath === 'python' || pythonPath === '/usr/bin/python';
17+
}

src/client/pythonEnvironments/legacyIOC.ts

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,17 @@ import { IServiceManager } from '../ioc/types';
3232
import { PythonEnvInfo, PythonEnvKind, PythonReleaseLevel } from './base/info';
3333
import { buildEnvInfo } from './base/info/env';
3434
import { ILocator, PythonLocatorQuery } from './base/locator';
35+
import { isMacDefaultPythonPath } from './base/locators/lowLevel/macDefaultLocator';
3536
import { getEnvs } from './base/locatorUtils';
37+
import { getEnvironmentDirFromPath } from './common/commonUtils';
3638
import { inExperiment } from './common/externalDependencies';
3739
import { PythonInterpreterLocatorService } from './discovery/locators';
3840
import { InterpreterLocatorHelper } from './discovery/locators/helpers';
3941
import { InterpreterLocatorProgressService } from './discovery/locators/progressService';
4042
import { CondaEnvironmentInfo } from './discovery/locators/services/conda';
4143
import { CondaEnvFileService } from './discovery/locators/services/condaEnvFileService';
4244
import { CondaEnvService } from './discovery/locators/services/condaEnvService';
45+
import { isCondaEnvironment } from './discovery/locators/services/condaLocator';
4346
import { CondaService } from './discovery/locators/services/condaService';
4447
import { CurrentPathService, PythonInPathCommandProvider } from './discovery/locators/services/currentPathService';
4548
import {
@@ -54,6 +57,7 @@ import { PipEnvService } from './discovery/locators/services/pipEnvService';
5457
import { PipEnvServiceHelper } from './discovery/locators/services/pipEnvServiceHelper';
5558
import { WindowsRegistryService } from './discovery/locators/services/windowsRegistryService';
5659
import { WindowsStoreInterpreter } from './discovery/locators/services/windowsStoreInterpreter';
60+
import { isWindowsStoreEnvironment } from './discovery/locators/services/windowsStoreLocator';
5761
import {
5862
WorkspaceVirtualEnvironmentsSearchPathProvider,
5963
WorkspaceVirtualEnvService,
@@ -189,11 +193,11 @@ class ComponentAdapter implements IComponentAdapter, IExtensionSingleActivationS
189193
if (!this.enabled) {
190194
return undefined;
191195
}
192-
const env = await this.api.resolveEnv(pythonPath);
193-
if (env === undefined) {
194-
return undefined;
195-
}
196-
return env.kind === PythonEnvKind.MacDefault;
196+
// While `ComponentAdapter` represents how the component would be used in the rest of the
197+
// extension, we cheat here for the sake of performance. This is not a problem because when
198+
// we start using the component's public API directly we will be dealing with `PythonEnvInfo`
199+
// instead of just `pythonPath`.
200+
return isMacDefaultPythonPath(pythonPath);
197201
}
198202

199203
// IInterpreterService
@@ -229,30 +233,30 @@ class ComponentAdapter implements IComponentAdapter, IExtensionSingleActivationS
229233
if (!this.enabled) {
230234
return undefined;
231235
}
232-
const env = await this.api.resolveEnv(interpreterPath);
233-
if (env === undefined) {
234-
return undefined;
235-
}
236-
return env.kind === PythonEnvKind.Conda;
236+
// While `ComponentAdapter` represents how the component would be used in the rest of the
237+
// extension, we cheat here for the sake of performance. This is not a problem because when
238+
// we start using the component's public API directly we will be dealing with `PythonEnvInfo`
239+
// instead of just `pythonPath`.
240+
return isCondaEnvironment(interpreterPath);
237241
}
238242

239243
// A result of `undefined` means "Fall back to the old code!"
240244
public async getCondaEnvironment(interpreterPath: string): Promise<CondaEnvironmentInfo | undefined> {
241245
if (!this.enabled) {
242246
return undefined;
243247
}
244-
const env = await this.api.resolveEnv(interpreterPath);
245-
if (env === undefined) {
246-
return undefined;
247-
}
248-
if (env.kind !== PythonEnvKind.Conda) {
248+
if (!(await isCondaEnvironment(interpreterPath))) {
249249
return undefined;
250250
}
251-
if (env.name !== '') {
252-
return { name: env.name, path: '' };
253-
}
251+
// For Conda we assume we don't set name for environments if they're prefix conda environments, similarly
252+
// we don't have 'path' set if they're non-prefix conda environments.
253+
// So we don't have a helper function yet to give us a conda env's name (if it has one). So for
254+
// now we always set `path` (and never `name`). Once we have such a helper we will use it.
255+
// tslint:disable-next-line:no-suspicious-comment
256+
// TODO: Expose these two properties via a helper in the Conda locator on a temporary basis.
257+
const location = getEnvironmentDirFromPath(interpreterPath);
254258
// else
255-
return { name: '', path: env.location };
259+
return { name: '', path: location };
256260
}
257261

258262
// IWindowsStoreInterpreter
@@ -262,11 +266,9 @@ class ComponentAdapter implements IComponentAdapter, IExtensionSingleActivationS
262266
if (!this.enabled) {
263267
return undefined;
264268
}
265-
const env = await this.api.resolveEnv(pythonPath);
266-
if (env) {
267-
return env.kind === PythonEnvKind.WindowsStore;
268-
}
269-
return undefined;
269+
// Eventually we won't be calling 'isWindowsStoreInterpreter' in the component adapter, so we won't
270+
// need to use 'isWindowsStoreEnvironment' directly here. This is just a temporary implementation.
271+
return isWindowsStoreEnvironment(pythonPath);
270272
}
271273

272274
// IInterpreterLocatorService

0 commit comments

Comments
 (0)