From b0dcbb5da8b436395775e8719b6027e677535ac1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 15 Jul 2021 10:03:53 -0700 Subject: [PATCH 1/3] List global python kernelspecs --- news/2 Fixes/6622.md | 1 + .../localKernelSpecFinderBase.ts | 5 ++ ...thonAndRelatedNonPythonKernelSpecFinder.ts | 58 +++++++++++++++++-- 3 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 news/2 Fixes/6622.md diff --git a/news/2 Fixes/6622.md b/news/2 Fixes/6622.md new file mode 100644 index 00000000000..10a1a2a9972 --- /dev/null +++ b/news/2 Fixes/6622.md @@ -0,0 +1 @@ +List non-traditional (not using `ipykernel`) global Python kernelspecs. diff --git a/src/client/datascience/kernel-launcher/localKernelSpecFinderBase.ts b/src/client/datascience/kernel-launcher/localKernelSpecFinderBase.ts index bfc7b6cb6d6..49cbf8a3ac6 100644 --- a/src/client/datascience/kernel-launcher/localKernelSpecFinderBase.ts +++ b/src/client/datascience/kernel-launcher/localKernelSpecFinderBase.ts @@ -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'; @@ -42,6 +43,10 @@ export abstract class LocalKernelSpecFinderBase { protected readonly extensionChecker: IPythonExtensionChecker ) {} + @testOnlyMethod() + public clearCache() { + 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 diff --git a/src/client/datascience/kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder.ts b/src/client/datascience/kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder.ts index cf804a6b237..3d316101b58 100644 --- a/src/client/datascience/kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder.ts +++ b/src/client/datascience/kernel-launcher/localPythonAndRelatedNonPythonKernelSpecFinder.ts @@ -75,7 +75,7 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS private async listGlobalPythonKernelSpecs( includeKernelsRegisteredByUs: boolean, cancelToken?: CancellationToken - ): Promise<(KernelSpecConnectionMetadata | PythonKernelConnectionMetadata)[]> { + ): Promise { const kernelSpecs = await this.kernelSpecsFromKnownLocations.listKernelSpecs(true, cancelToken); return ( kernelSpecs @@ -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) => item) ); } /** @@ -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]; @@ -125,6 +144,31 @@ export class LocalPythonAndRelatedNonPythonKernelSpecFinder extends LocalKernelS // Then go through all of the kernels and generate their metadata const distinctKernelMetadata = new Map(); + + // 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) => { @@ -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; From 1776c0f85acd4a9d73ddddaad10c0833236beb46 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 15 Jul 2021 10:14:57 -0700 Subject: [PATCH 2/3] misc --- .../notebook/intellisense/completionProvider.vscode.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts b/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts index 76ddd26f31d..94151fe36dd 100644 --- a/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts +++ b/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts @@ -88,6 +88,8 @@ suite('DataScience - VSCode Notebook - (Code Completion via Jupyter) (slow)', fu const completions = await completionProvider.provideCompletionItems(cell2.document, position, token, context); console.log(JSON.stringify(completions)); const items = completions.map((item) => item.label); + console.error('items.forEach'); + items.forEach((item) => (typeof item === 'string' ? console.error(item) : console.error(item.label))); assert.isOk(items.length); assert.include(items, 'bit_length'); assert.include(items, 'to_bytes'); From 2ef68f1f9e749a00debc842da3bc0b66283bfc07 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 15 Jul 2021 10:47:05 -0700 Subject: [PATCH 3/3] fix tests --- .../completionProvider.vscode.test.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts b/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts index 94151fe36dd..e2f391f2202 100644 --- a/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts +++ b/src/test/datascience/notebook/intellisense/completionProvider.vscode.test.ts @@ -86,12 +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); - console.error('items.forEach'); - items.forEach((item) => (typeof item === 'string' ? console.error(item) : console.error(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') + ) + ); + assert.ok( + items.find((item) => + typeof item === 'string' ? item.includes('to_bytes') : item.label.includes('to_bytes') + ) + ); }); });