Select kernel based on metadata in notebook (#14217)

news/2 Fixes/14213.md

@@ -0,0 +1 @@
+Use the kernel defined in the metadata of Notebook instead of using the default workspace interpreter.

src/client/datascience/jupyter/kernels/helpers.ts

@@ -25,8 +25,6 @@ import {
 
 // Helper functions for dealing with kernels and kernelspecs
 
-export const defaultKernelSpecName = 'python_defaultSpec_';
-
 // https://jupyter-client.readthedocs.io/en/stable/kernels.html
 const connectionFilePlaceholder = '{connection_file}';
 
@@ -136,13 +134,13 @@ export function getKernelConnectionLanguage(kernelConnection?: KernelConnectionM
     return model?.language || kernelSpec?.language;
 }
 // Create a default kernelspec with the given display name
-export function createDefaultKernelSpec(displayName?: string): IJupyterKernelSpec {
+export function createDefaultKernelSpec(interpreter?: PythonEnvironment): IJupyterKernelSpec {
     // This creates a default kernel spec. When launched, 'python' argument will map to using the interpreter
     // associated with the current resource for launching.
     const defaultSpec: Kernel.ISpecModel = {
-        name: defaultKernelSpecName + Date.now().toString(),
+        name: interpreter?.displayName || 'Python 3',
         language: 'python',
-        display_name: displayName || 'Python 3',
+        display_name: interpreter?.displayName || 'Python 3',
         metadata: {},
         argv: ['python', '-m', 'ipykernel_launcher', '-f', connectionFilePlaceholder],
         env: {},

src/client/datascience/jupyter/kernels/kernelSelector.ts

@@ -549,7 +549,7 @@ export class KernelSelector implements IKernelSelectionUsage {
 
     // When switching to an interpreter in raw kernel mode then just create a default kernelspec for that interpreter to use
     private async useInterpreterAndDefaultKernel(interpreter: PythonEnvironment): Promise<KernelConnectionMetadata> {
-        const kernelSpec = createDefaultKernelSpec(interpreter.displayName);
+        const kernelSpec = createDefaultKernelSpec(interpreter);
         return { kernelSpec, interpreter, kind: 'startUsingPythonInterpreter' };
     }
 

src/client/datascience/jupyter/kernels/kernelSwitcher.ts

@@ -90,7 +90,10 @@ export class KernelSwitcher {
         const kernelSpec = kernelConnectionMetadataHasKernelSpec(kernelConnection) ? kernelConnection : undefined;
         const kernelName = kernelSpec?.kernelSpec?.name || kernelModel?.kernelModel?.name;
         // One of them is bound to be non-empty.
-        const displayName = kernelModel?.kernelModel?.display_name || kernelName || '';
+        const displayName =
+            kernelConnection.kind === 'startUsingPythonInterpreter'
+                ? kernelConnection.interpreter.displayName || kernelConnection.interpreter.path
+                : kernelModel?.kernelModel?.display_name || kernelName || '';
         const options: ProgressOptions = {
             location: ProgressLocation.Notification,
             cancellable: false,

src/client/datascience/kernel-launcher/kernelFinder.ts

@@ -16,7 +16,6 @@ import { IInterpreterLocatorService, IInterpreterService, KNOWN_PATH_SERVICE } f
 import { captureTelemetry } from '../../telemetry';
 import { getRealPath } from '../common';
 import { Telemetry } from '../constants';
-import { defaultKernelSpecName } from '../jupyter/kernels/helpers';
 import { JupyterKernelSpec } from '../jupyter/kernels/jupyterKernelSpec';
 import { IDataScienceFileSystem, IJupyterKernelSpec } from '../types';
 import { IKernelFinder } from './types';
@@ -70,35 +69,32 @@ export class KernelFinder implements IKernelFinder {
         const kernelName = kernelSpecMetadata?.name;
 
         if (kernelSpecMetadata && kernelName) {
-            // For a non default kernelspec search for it
-            if (!kernelName.includes(defaultKernelSpecName)) {
-                let kernelSpec = await this.searchCache(kernelName);
+            let kernelSpec = await this.searchCache(kernelName);
 
-                if (kernelSpec) {
-                    return kernelSpec;
-                }
+            if (kernelSpec) {
+                return kernelSpec;
+            }
 
-                // Check in active interpreter first
-                kernelSpec = await this.getKernelSpecFromActiveInterpreter(kernelName, resource);
+            // Check in active interpreter first
+            kernelSpec = await this.getKernelSpecFromActiveInterpreter(kernelName, resource);
 
-                if (kernelSpec) {
-                    this.writeCache().ignoreErrors();
-                    return kernelSpec;
-                }
-
-                const diskSearch = this.findDiskPath(kernelName);
-                const interpreterSearch = this.getInterpreterPaths(resource).then((interpreterPaths) => {
-                    return this.findInterpreterPath(interpreterPaths, kernelName);
-                });
+            if (kernelSpec) {
+                this.writeCache().ignoreErrors();
+                return kernelSpec;
+            }
 
-                let result = await Promise.race([diskSearch, interpreterSearch]);
-                if (!result) {
-                    const both = await Promise.all([diskSearch, interpreterSearch]);
-                    result = both[0] ? both[0] : both[1];
-                }
+            const diskSearch = this.findDiskPath(kernelName);
+            const interpreterSearch = this.getInterpreterPaths(resource).then((interpreterPaths) => {
+                return this.findInterpreterPath(interpreterPaths, kernelName);
+            });
 
-                foundKernel = result;
+            let result = await Promise.race([diskSearch, interpreterSearch]);
+            if (!result) {
+                const both = await Promise.all([diskSearch, interpreterSearch]);
+                result = both[0] ? both[0] : both[1];
             }
+
+            foundKernel = result;
         }
 
         this.writeCache().ignoreErrors();

src/client/datascience/kernel-launcher/kernelProcess.ts

@@ -151,10 +151,7 @@ export class KernelProcess implements IKernelProcess {
         let kernelSpec = this._kernelConnectionMetadata.kernelSpec;
         // If there is no kernelspec & when launching a Python process, generate a dummy `kernelSpec`
         if (!kernelSpec && this._kernelConnectionMetadata.kind === 'startUsingPythonInterpreter') {
-            kernelSpec = createDefaultKernelSpec(
-                this._kernelConnectionMetadata.interpreter.displayName ||
-                    this._kernelConnectionMetadata.interpreter.path
-            );
+            kernelSpec = createDefaultKernelSpec(this._kernelConnectionMetadata.interpreter);
         }
         // We always expect a kernel spec.
         if (!kernelSpec) {

src/client/datascience/notebookStorage/baseModel.ts

@@ -56,7 +56,22 @@ export function updateNotebookMetadata(
         kernelConnection && kernelConnectionMetadataHasKernelModel(kernelConnection)
             ? kernelConnection.kernelModel
             : kernelConnection?.kernelSpec;
-    if (kernelSpecOrModel && !metadata.kernelspec) {
+    if (kernelConnection?.kind === 'startUsingPythonInterpreter') {
+        // Store interpreter name, we expect the kernel finder will find the corresponding interpreter based on this name.
+        const name = kernelConnection.interpreter.displayName || '';
+        if (metadata.kernelspec?.name !== name || metadata.kernelspec?.display_name !== name) {
+            changed = true;
+            metadata.kernelspec = {
+                name,
+                display_name: name,
+                metadata: {
+                    interpreter: {
+                        hash: sha256().update(kernelConnection.interpreter.path).digest('hex')
+                    }
+                }
+            };
+        }
+    } else if (kernelSpecOrModel && !metadata.kernelspec) {
         // Add a new spec in this case
         metadata.kernelspec = {
             name: kernelSpecOrModel.name || kernelSpecOrModel.display_name || '',
@@ -78,20 +93,12 @@ export function updateNotebookMetadata(
             metadata.kernelspec.display_name = displayName;
             kernelId = kernelSpecOrModel.id;
         }
-    } else if (kernelConnection?.kind === 'startUsingPythonInterpreter') {
-        // Store interpreter name, we expect the kernel finder will find the corresponding interpreter based on this name.
-        const name = kernelConnection.interpreter.displayName || kernelConnection.interpreter.path;
-        if (metadata.kernelspec?.name !== name || metadata.kernelspec?.display_name !== name) {
-            changed = true;
-            metadata.kernelspec = {
-                name,
-                display_name: name,
-                metadata: {
-                    interpreter: {
-                        hash: sha256().update(kernelConnection.interpreter.path).digest('hex')
-                    }
-                }
-            };
+        try {
+            // This is set only for when we select an interpreter.
+            // tslint:disable-next-line: no-any
+            delete (metadata.kernelspec as any).metadata;
+        } catch {
+            // Noop.
         }
     }
     return { changed, kernelId };

src/test/datascience/jupyter/kernels/kernelSelector.unit.test.ts

@@ -19,7 +19,6 @@ import { Architecture } from '../../../../client/common/utils/platform';
 import { StopWatch } from '../../../../client/common/utils/stopWatch';
 import { JupyterSessionManager } from '../../../../client/datascience/jupyter/jupyterSessionManager';
 import { JupyterSessionManagerFactory } from '../../../../client/datascience/jupyter/jupyterSessionManagerFactory';
-import { defaultKernelSpecName } from '../../../../client/datascience/jupyter/kernels/helpers';
 import { KernelDependencyService } from '../../../../client/datascience/jupyter/kernels/kernelDependencyService';
 import { KernelSelectionProvider } from '../../../../client/datascience/jupyter/kernels/kernelSelections';
 import { KernelSelector } from '../../../../client/datascience/jupyter/kernels/kernelSelector';
@@ -453,9 +452,6 @@ suite('DataScience - KernelSelector', () => {
 
             assert.deepEqual(kernel?.interpreter, interpreter);
             expect((kernel as any)?.kernelSpec, 'Should have kernelspec').to.not.be.undefined;
-            expect((kernel as any)?.kernelSpec!.name, 'Spec should have default name').to.include(
-                defaultKernelSpecName
-            );
         });
         test('For a raw connection, if a kernel spec is selected return it with the interpreter', async () => {
             when(dependencyService.areDependenciesInstalled(interpreter, anything())).thenResolve(true);

src/test/datascience/kernelFinder.unit.test.ts

@@ -14,7 +14,6 @@ import { PythonExecutionFactory } from '../../client/common/process/pythonExecut
 import { IExtensionContext, IPathUtils, Resource } from '../../client/common/types';
 import { Architecture } from '../../client/common/utils/platform';
 import { IEnvironmentVariablesProvider } from '../../client/common/variables/types';
-import { defaultKernelSpecName } from '../../client/datascience/jupyter/kernels/helpers';
 import { JupyterKernelSpec } from '../../client/datascience/jupyter/kernels/jupyterKernelSpec';
 import { KernelFinder } from '../../client/datascience/kernel-launcher/kernelFinder';
 import { IKernelFinder } from '../../client/datascience/kernel-launcher/types';
@@ -526,25 +525,6 @@ suite('Kernel Finder', () => {
             fileSystem.reset();
         });
 
-        test('Kernel metadata already has a default spec, and kernel spec not found, then return undefined', async () => {
-            setupFileSystem();
-            fileSystem
-                .setup((fs) => fs.readLocalFile(typemoq.It.isAnyString()))
-                .returns((pathParam: string) => {
-                    if (pathParam.includes(cacheFile)) {
-                        return Promise.resolve('[]');
-                    }
-                    return Promise.resolve('{}');
-                });
-            // get default kernel
-            const spec = await kernelFinder.findKernelSpec(resource, {
-                name: defaultKernelSpecName,
-                display_name: 'TargetDisplayName'
-            });
-            assert.isUndefined(spec);
-            fileSystem.reset();
-        });
-
         test('Look for KernelA with no cache, find KernelA and KenelB, then search for KernelB and find it in cache', async () => {
             setupFileSystem();
             fileSystem