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.

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

List global python kernelspecs #6676

Merged
merged 3 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions news/2 Fixes/6622.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
List non-traditional (not using `ipykernel`) global Python kernelspecs.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { PYTHON_LANGUAGE } from '../../common/constants';
import { traceDecorators, traceError, traceInfo, traceInfoIf } from '../../common/logger';
import { IFileSystem } from '../../common/platform/types';
import { ReadWrite } from '../../common/types';
import { testOnlyMethod } from '../../common/utils/decorators';
import { PythonEnvironment } from '../../pythonEnvironments/info';
import { getInterpreterKernelSpecName } from '../jupyter/kernels/helpers';
import { JupyterKernelSpec } from '../jupyter/kernels/jupyterKernelSpec';
Expand Down Expand Up @@ -42,6 +43,10 @@ export abstract class LocalKernelSpecFinderBase {
protected readonly extensionChecker: IPythonExtensionChecker
) {}

@testOnlyMethod()
public clearCache() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add tests later, after i finish ipykernel6 stuff (seprate PR)

this.kernelSpecCache.clear();
}
/**
* @param {boolean} dependsOnPythonExtension Whether this list of kernels fetched depends on whether the python extension is installed/not installed.
* If for instance first Python Extension isn't installed, then we call this again, after installing it, then the cache will be blown away
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
private async listGlobalPythonKernelSpecs(
includeKernelsRegisteredByUs: boolean,
cancelToken?: CancellationToken
): Promise<(KernelSpecConnectionMetadata | PythonKernelConnectionMetadata)[]> {
): Promise<KernelSpecConnectionMetadata[]> {
const kernelSpecs = await this.kernelSpecsFromKnownLocations.listKernelSpecs(true, cancelToken);
return (
kernelSpecs
Expand All @@ -84,6 +84,7 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
// Those were registered by us to start kernels from Jupyter extension (not stuff that user created).
// We should only return global kernels the user created themselves, others will appear when searching for interprters.
.filter((item) => (includeKernelsRegisteredByUs ? true : !isKernelRegisteredByUs(item.kernelSpec)))
.map((item) => <KernelSpecConnectionMetadata>item)
);
}
/**
Expand Down Expand Up @@ -113,6 +114,24 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
const globalPythonKernelSpecsRegisteredByUs = globalKernelSpecs.filter((item) =>
isKernelRegisteredByUs(item.kernelSpec)
);
// Possible there are Python kernels (language=python, but not necessarily using ipykernel).
// E.g. cadabra2 is one such kernel (similar to powershell kernel but language is still python).
const usingNonIpyKernelLauncher = (item: KernelSpecConnectionMetadata | PythonKernelConnectionMetadata) => {
if (item.kernelSpec.language !== PYTHON_LANGUAGE) {
return false;
}
const args = item.kernelSpec.argv.map((arg) => arg.toLowerCase());
const moduleIndex = args.indexOf('-m');
if (moduleIndex === -1) {
return false;
}
const moduleName = args.length - 1 >= moduleIndex ? args[moduleIndex + 1] : undefined;
if (!moduleName) {
return false;
}
// We are only interested in global kernels that don't use ipykernel_launcher.
return moduleName !== 'ipykernel_launcher';
};
// Copy the interpreter list. We need to filter out those items
// which have matched one or more kernelspecs
let filteredInterpreters = [...interpreters];
Expand All @@ -125,6 +144,31 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS

// Then go through all of the kernels and generate their metadata
const distinctKernelMetadata = new Map<string, KernelSpecConnectionMetadata | PythonKernelConnectionMetadata>();

// Go through the global kernelspecs that use python to launch the kernel but don't use ipykernel.
globalKernelSpecs
.filter((item) => !isKernelRegisteredByUs(item.kernelSpec) && usingNonIpyKernelLauncher(item))
.forEach((item) => {
// If we cannot find a matching interpreter, then too bad.
// We can't use any interpreter, because the module used is not `ipykernel_laucnher`.
// Its something special, hence ignore if we cannot find a matching interpreter.
const matchingInterpreter = this.findMatchingInterpreter(item.kernelSpec, interpreters);
if (!matchingInterpreter) {
traceInfo(
`Kernel Spec for ${
item.kernelSpec.display_name
} ignored as we cannot find a matching interpreter ${JSON.stringify(item)}`
);
return;
}
const kernelSpec: KernelSpecConnectionMetadata = {
kind: 'startUsingKernelSpec',
kernelSpec: item.kernelSpec,
interpreter: matchingInterpreter,
id: getKernelId(item.kernelSpec, matchingInterpreter)
};
distinctKernelMetadata.set(kernelSpec.id, kernelSpec);
});
await Promise.all(
[...kernelSpecs, ...globalPythonKernelSpecsRegisteredByUs.map((item) => item.kernelSpec)]
.filter((kernelspec) => {
Expand All @@ -151,9 +195,15 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS
id: getKernelId(k, matchingInterpreter)
};

// If interpreters were found, remove them from the interpreter list we'll eventually
// return as interpreter only items
filteredInterpreters = filteredInterpreters.filter((i) => matchingInterpreter !== i);
// Hide the interpreters from list of kernels only if this kernel is not something the user created.
// Users can create their own kernels with custom environment variables, in such cases, we should list that
// kernel as well as the interpreter (so they can use both).
const isUserCreatedKernel =
!isKernelRegisteredByUs(result.kernelSpec) &&
Object.keys(result.kernelSpec.env || {}).length > 0;
if (!isUserCreatedKernel) {
filteredInterpreters = filteredInterpreters.filter((i) => matchingInterpreter !== i);
}

// Return our metadata that uses an interpreter to start
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,17 @@ suite('DataScience - VSCode Notebook - (Code Completion via Jupyter) (slow)', fu
};
traceInfo('Get completions in test');
const completions = await completionProvider.provideCompletionItems(cell2.document, position, token, context);
console.log(JSON.stringify(completions));
const items = completions.map((item) => item.label);
assert.isOk(items.length);
assert.include(items, 'bit_length');
assert.include(items, 'to_bytes');
assert.ok(
items.find((item) =>
typeof item === 'string' ? item.includes('bit_length') : item.label.includes('bit_length')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes tests with failing code completion (this is a bug in jupyter, changes are breaking us, the return value is different)
We already have an issue to fix the underlying bug (this way this test passes)

)
);
assert.ok(
items.find((item) =>
typeof item === 'string' ? item.includes('to_bytes') : item.label.includes('to_bytes')
)
);
});
});