Skip to content
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

reduce usages of resolveEnvironments #15257

Merged
merged 5 commits into from
Feb 28, 2024
Merged

reduce usages of resolveEnvironments #15257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd3fd0c
reduce usages of resolveEnvironments
DonJayamanne Feb 28, 2024
893b6ab
Removals
DonJayamanne Feb 28, 2024
f53299b
Reduce usage of resolveEnvs from more places
DonJayamanne Feb 28, 2024
2876547
Remove more usages
DonJayamanne Feb 28, 2024
4e0ba79
oops
DonJayamanne Feb 28, 2024
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
8 changes: 6 additions & 2 deletions src/kernels/helpers.unit.test.ts
View file
Open in desktop
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
1 change: 0 additions & 1 deletion src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts
View file
Open in desktop
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
View file
Open in desktop
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
Loading