Skip to content

Commit

Permalink
reduce usages of resolveEnvironments (#15257)
Browse files Browse the repository at this point in the history
* reduce usages of resolveEnvironments

* Removals

* Reduce usage of resolveEnvs from more places

* Remove more usages

* oops
  • Loading branch information
DonJayamanne authored Feb 28, 2024
1 parent d331387 commit 024a361
Show file tree
Hide file tree
Showing 14 changed files with 430 additions and 271 deletions.
8 changes: 6 additions & 2 deletions src/kernels/helpers.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { setPythonApi } from '../platform/interpreter/helpers';
import type { IDisposable } from '@c4312/evt';
import type { DeepPartial } from '../platform/common/utils/misc';

export function crateMockedPythonApi(disposables: IDisposable[]) {
export function crateMockedPythonApi(disposables: IDisposable[] | DisposableStore) {
const disposableStore = new DisposableStore();
const mockedApi = mock<PythonExtension>();
sinon.stub(PythonExtension, 'api').resolves(resolvableInstance(mockedApi));
Expand All @@ -30,7 +30,11 @@ export function crateMockedPythonApi(disposables: IDisposable[]) {
when(environments.known).thenReturn([]);
setPythonApi(instance(mockedApi));
disposableStore.add({ dispose: () => setPythonApi(undefined as any) });
disposables.push(disposableStore);
if (Array.isArray(disposables)) {
disposables.push(disposableStore);
} else {
disposables.add(disposableStore);
}
return { dispose: () => disposableStore.dispose(), environments };
}
export function whenKnownEnvironments(environments: PythonExtension['environments']) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ import { setPythonApi } from '../../../platform/interpreter/helpers';
when(interpreterService.onDidChangeInterpreters).thenReturn(onDidChangeInterpreters.event);
when(interpreterService.onDidRemoveInterpreter).thenReturn(onDidDeleteInterpreter.event);
when(interpreterService.onDidChangeStatus).thenReturn(onDidChangeInterpreterStatus.event);
when(interpreterService.resolvedEnvironments).thenReturn(Array.from(distinctInterpreters));
when(interpreterService.getActiveInterpreter(anything())).thenResolve(activeInterpreter);
when(interpreterService.getInterpreterDetails(anything())).thenResolve();
when(interpreterService.getInterpreterDetails(anything(), anything())).thenResolve();
Expand Down
61 changes: 33 additions & 28 deletions src/kernels/raw/finder/interpreterKernelSpecFinderHelper.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ import { LocalKnownPathKernelSpecFinder } from './localKnownPathKernelSpecFinder
import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc';
import {
getCachedEnvironment,
getCachedEnvironments,
getCachedSysPrefix,
getCachedVersion,
getEnvironmentType,
getSysPrefix
getSysPrefix,
resolvedPythonEnvToJupyterEnv
} from '../../../platform/interpreter/helpers';
import { Environment } from '@vscode/python-extension';

Expand Down Expand Up @@ -362,9 +364,7 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
kernelConnectionType: KernelConnectionMetadata['kind'],
cancelToken?: CancellationToken
): Promise<PythonEnvironment | undefined> {
const interpreters = this.extensionChecker.isPythonExtensionInstalled
? this.interpreterService.resolvedEnvironments
: [];
const interpreters = this.extensionChecker.isPythonExtensionInstalled ? getCachedEnvironments() : [];

const pathInArgv =
kernelSpec && Array.isArray(kernelSpec.argv) && kernelSpec.argv.length > 0 ? kernelSpec.argv[0] : undefined;
Expand All @@ -391,7 +391,7 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
const exactMatch = interpreters.find((i) => {
if (
kernelSpec.metadata?.interpreter?.path &&
areInterpreterPathsSame(Uri.file(kernelSpec.metadata.interpreter.path), i.uri)
areInterpreterPathsSame(Uri.file(kernelSpec.metadata.interpreter.path), i.executable.uri)
) {
traceVerbose(
`Kernel ${kernelSpec.name} matches ${getDisplayPath(i.id)} based on metadata.interpreter.`
Expand All @@ -401,7 +401,7 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
return false;
});
if (exactMatch) {
return exactMatch;
return resolvedPythonEnvToJupyterEnv(exactMatch);
}
if (pathInArgv && path.basename(pathInArgv) === pathInArgv && kernelSpec.specFile && !isCreatedByUs) {
sendTelemetryEvent(Telemetry.AmbiguousGlobalKernelSpec, undefined, {
Expand All @@ -417,7 +417,7 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
if (pathInArgv && path.basename(pathInArgv) !== pathInArgv) {
const pathInArgVUri = Uri.file(pathInArgv);
const exactMatchBasedOnArgv = interpreters.find((i) => {
if (areInterpreterPathsSame(pathInArgVUri, i.uri)) {
if (areInterpreterPathsSame(pathInArgVUri, i.executable.uri)) {
traceVerbose(`Kernel ${kernelSpec.name} matches ${getDisplayPath(i.id)} based on argv.`);
return true;
}
Expand All @@ -435,7 +435,7 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
isCreatedByUs
});
}
return exactMatchBasedOnArgv;
return resolvedPythonEnvToJupyterEnv(exactMatchBasedOnArgv);
}

// 3. Sometimes we have path paths such as `/usr/bin/python3.6` in the kernel spec.
Expand Down Expand Up @@ -491,14 +491,17 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
if (kernelSpec.interpreterPath) {
const kernelSpecInterpreterPath = Uri.file(kernelSpec.interpreterPath);
const matchBasedOnInterpreterPath = interpreters.find((i) => {
if (kernelSpec.interpreterPath && areInterpreterPathsSame(kernelSpecInterpreterPath, i.uri)) {
if (
kernelSpec.interpreterPath &&
areInterpreterPathsSame(kernelSpecInterpreterPath, i.executable.uri)
) {
traceVerbose(`Kernel ${kernelSpec.name} matches ${getDisplayPath(i.id)} based on interpreterPath.`);
return true;
}
return false;
});
if (matchBasedOnInterpreterPath) {
return matchBasedOnInterpreterPath;
return resolvedPythonEnvToJupyterEnv(matchBasedOnInterpreterPath);
}
// Possible we still haven't discovered this interpreter, hence get the details from the Python extension.
if (!kernelSpec.specFile || this.trustedKernels.isTrusted(Uri.file(kernelSpec.specFile))) {
Expand All @@ -515,24 +518,26 @@ export class GlobalPythonKernelSpecFinder implements IDisposable {
return;
}

return interpreters.find((i) => {
// 4. Check display name
if (kernelSpec.display_name === getCachedEnvironment(i)?.environment?.name) {
traceVerbose(`Kernel ${kernelSpec.name} matches ${getDisplayPath(i.id)} based on display name`);
// This is a bad one, matching by name is never going to be accurate
sendTelemetryEvent(Telemetry.AmbiguousGlobalKernelSpec, undefined, {
kernelSpecHash,
kernelConnectionType,
pythonPathDefined: true,
argv0: pathInArgv ? path.basename(pathInArgv) : '',
pythonEnvFound: 'matchDisplayName',
language: kernelSpecLanguage,
isCreatedByUs
});
return true;
}
return false;
});
return resolvedPythonEnvToJupyterEnv(
interpreters.find((i) => {
// 4. Check display name
if (kernelSpec.display_name === getCachedEnvironment(i)?.environment?.name) {
traceVerbose(`Kernel ${kernelSpec.name} matches ${getDisplayPath(i.id)} based on display name`);
// This is a bad one, matching by name is never going to be accurate
sendTelemetryEvent(Telemetry.AmbiguousGlobalKernelSpec, undefined, {
kernelSpecHash,
kernelConnectionType,
pythonPathDefined: true,
argv0: pathInArgv ? path.basename(pathInArgv) : '',
pythonEnvFound: 'matchDisplayName',
language: kernelSpecLanguage,
isCreatedByUs
});
return true;
}
return false;
})
);
}
private listGlobalPythonKernelSpecs(): LocalKernelSpecConnectionMetadata[] {
return (this.lastKnownGlobalPythonKernelSpecs = this.kernelSpecsFromKnownLocations.kernels.filter(
Expand Down
Loading

0 comments on commit 024a361

Please sign in to comment.