Fixes to blowing away of kernel info & not using right startup info (#14295)

Author: DonJayamanne

build/ci/templates/test_phases.yml

@@ -118,7 +118,7 @@ steps:
       python -c "import sys;print(sys.executable)"
     displayName: 'pip install functional requirements'
     condition: and(succeeded(), eq(variables['NeedsPythonFunctionalReqs'], 'true'))
-    
+
   # Add CONDA to the path so anaconda works
   #
   # This task will only run if variable `NeedsPythonFunctionalReqs` is true.
@@ -422,7 +422,7 @@ steps:
   # Upload the test results to Azure DevOps to facilitate test reporting in their UX.
   - task: PublishTestResults@2
     displayName: 'Publish functional tests results'
-    condition: or(contains(variables['TestsToRun'], 'testFunctional'), contains(variables['TestsToRun'], 'testParallelFunctional')) 
+    condition: or(contains(variables['TestsToRun'], 'testFunctional'), contains(variables['TestsToRun'], 'testParallelFunctional'))
     inputs:
       testResultsFiles: '$(Build.ArtifactStagingDirectory)/test-junit*.xml'
       testRunTitle: 'functional-$(Agent.Os)-Py$(pythonVersion)'
@@ -446,7 +446,7 @@ steps:
   - script: |
       npm run testDataScience
     continueOnError: true
-    displayName: 'Run DataScience Tests in VSCode Insiders'
+    displayName: 'Run Native Notebook Tests in VSCode Insiders'
     condition: and(succeeded(), contains(variables['TestsToRun'], 'testDataScience'), not(contains(variables['TestsToRun'], 'testDataScienceInVSCode')))
     env:
       DISPLAY: :10

build/ci/vscode-python-pr-validation.yaml

@@ -52,7 +52,7 @@ stages:
             'Single Workspace':
               TestsToRun: 'testSingleWorkspace'
               NeedsPythonTestReqs: true
-            'DataScience':
+            'Native Notebook':
               TestsToRun: 'testDataScience'
               NeedsPythonTestReqs: true
               NeedsPythonFunctionalReqs: true

src/client/datascience/baseJupyterSession.ts

@@ -122,7 +122,12 @@ export abstract class BaseJupyterSession implements IJupyterSession {
             );
         }
     }
-
+    public async requestKernelInfo(): Promise<KernelMessage.IInfoReplyMsg> {
+        if (!this.session) {
+            throw new Error('Cannot request KernelInfo, Session not initialized.');
+        }
+        return this.session.kernel.requestKernelInfo();
+    }
     public async changeKernel(kernelConnection: KernelConnectionMetadata, timeoutMS: number): Promise<void> {
         let newSession: ISessionWithSocket | undefined;
 

src/client/datascience/constants.ts

@@ -574,6 +574,7 @@ export namespace LiveShareCommands {
     export const getUsableJupyterPython = 'getUsableJupyterPython';
     export const executeObservable = 'executeObservable';
     export const getSysInfo = 'getSysInfo';
+    export const requestKernelInfo = 'requestKernelInfo';
     export const serverResponse = 'serverResponse';
     export const catchupRequest = 'catchupRequest';
     export const syncRequest = 'synchRequest';

src/client/datascience/jupyter/jupyterNotebook.ts

@@ -241,7 +241,9 @@ export class JupyterNotebookBase implements INotebook {
             }
         }
     }
-
+    public async requestKernelInfo(): Promise<KernelMessage.IInfoReplyMsg> {
+        return this.session.requestKernelInfo();
+    }
     public get onSessionStatusChanged(): Event<ServerStatus> {
         if (!this.onStatusChangedEvent) {
             this.onStatusChangedEvent = new EventEmitter<ServerStatus>();

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

@@ -4,6 +4,7 @@
 'use strict';
 
 import { nbformat } from '@jupyterlab/coreutils';
+import { KernelMessage } from '@jupyterlab/services';
 import { Observable } from 'rxjs/Observable';
 import { Subject } from 'rxjs/Subject';
 import * as uuid from 'uuid/v4';
@@ -18,11 +19,10 @@ import {
 } from 'vscode';
 import { ServerStatus } from '../../../../datascience-ui/interactive-common/mainState';
 import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../../common/application/types';
-import { traceError } from '../../../common/logger';
+import { traceError, traceWarning } from '../../../common/logger';
 import { IDisposableRegistry } from '../../../common/types';
 import { createDeferred, Deferred } from '../../../common/utils/async';
 import { noop } from '../../../common/utils/misc';
-import { IInterpreterService } from '../../../interpreter/contracts';
 import { CodeSnippets } from '../../constants';
 import { getDefaultNotebookContent, updateNotebookMetadata } from '../../notebookStorage/baseModel';
 import {
@@ -51,6 +51,10 @@ export class Kernel implements IKernel {
     get onDisposed(): Event<void> {
         return this._onDisposed.event;
     }
+    private _info?: KernelMessage.IInfoReplyMsg['content'];
+    get info(): KernelMessage.IInfoReplyMsg['content'] | undefined {
+        return this._info;
+    }
     get status(): ServerStatus {
         return this.notebook?.status ?? ServerStatus.NotStarted;
     }
@@ -79,7 +83,6 @@ export class Kernel implements IKernel {
         private readonly disposables: IDisposableRegistry,
         private readonly launchTimeout: number,
         commandManager: ICommandManager,
-        interpreterService: IInterpreterService,
         private readonly errorHandler: IDataScienceErrorHandler,
         editorProvider: INotebookEditorProvider,
         private readonly kernelProvider: IKernelProvider,
@@ -90,12 +93,12 @@ export class Kernel implements IKernel {
         this.kernelExecution = new KernelExecution(
             kernelProvider,
             commandManager,
-            interpreterService,
             errorHandler,
             editorProvider,
             kernelSelectionUsage,
             appShell,
-            vscNotebook
+            vscNotebook,
+            metadata
         );
     }
     public async executeCell(cell: NotebookCell): Promise<void> {
@@ -124,8 +127,9 @@ export class Kernel implements IKernel {
         } else {
             await this.validate(this.uri);
             const metadata = ((getDefaultNotebookContent().metadata || {}) as unknown) as nbformat.INotebookMetadata;
+            // Create a dummy notebook metadata & update the metadata before starting the notebook (required to ensure we fetch & start the right kernel).
+            // Lower layers of code below getOrCreateNotebook searches for kernels again using the metadata.
             updateNotebookMetadata(metadata, this.metadata);
-
             this._notebookPromise = this.notebookProvider.getOrCreateNotebook({
                 identity: this.uri,
                 resource: this.uri,
@@ -232,6 +236,10 @@ export class Kernel implements IKernel {
         if (isPythonKernelConnection(this.metadata)) {
             await this.notebook.setLaunchingFile(this.uri.fsPath);
         }
+        await this.notebook
+            .requestKernelInfo()
+            .then((item) => (this._info = item.content))
+            .catch(traceWarning.bind('Failed to request KernelInfo'));
         await this.notebook.waitForIdle(this.launchTimeout);
     }
 

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

@@ -7,13 +7,12 @@ import { NotebookCell, NotebookCellRunState, NotebookDocument } from 'vscode';
 import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../../common/application/types';
 import { IDisposable } from '../../../common/types';
 import { noop } from '../../../common/utils/misc';
-import { IInterpreterService } from '../../../interpreter/contracts';
 import { captureTelemetry } from '../../../telemetry';
 import { Commands, Telemetry, VSCodeNativeTelemetry } from '../../constants';
 import { MultiCancellationTokenSource } from '../../notebook/helpers/multiCancellationToken';
 import { IDataScienceErrorHandler, INotebook, INotebookEditorProvider } from '../../types';
 import { CellExecution, CellExecutionFactory } from './cellExecution';
-import type { IKernel, IKernelProvider, IKernelSelectionUsage } from './types';
+import type { IKernel, IKernelProvider, IKernelSelectionUsage, KernelConnectionMetadata } from './types';
 // tslint:disable-next-line: no-var-requires no-require-imports
 const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
 
@@ -35,12 +34,12 @@ export class KernelExecution implements IDisposable {
     constructor(
         private readonly kernelProvider: IKernelProvider,
         private readonly commandManager: ICommandManager,
-        private readonly interpreterService: IInterpreterService,
         errorHandler: IDataScienceErrorHandler,
         editorProvider: INotebookEditorProvider,
         readonly kernelSelectionUsage: IKernelSelectionUsage,
         readonly appShell: IApplicationShell,
-        readonly vscNotebook: IVSCodeNotebook
+        readonly vscNotebook: IVSCodeNotebook,
+        readonly metadata: Readonly<KernelConnectionMetadata>
     ) {
         this.executionFactory = new CellExecutionFactory(errorHandler, editorProvider, appShell, vscNotebook);
     }
@@ -142,15 +141,7 @@ export class KernelExecution implements IDisposable {
         await this.validateKernel(document);
         let kernel = this.kernelProvider.get(document.uri);
         if (!kernel) {
-            const activeInterpreter = await this.interpreterService.getActiveInterpreter(document.uri);
-            kernel = this.kernelProvider.getOrCreate(document.uri, {
-                metadata: {
-                    interpreter: activeInterpreter!,
-                    kernelModel: undefined,
-                    kernelSpec: undefined,
-                    kind: 'startUsingPythonInterpreter'
-                }
-            });
+            kernel = this.kernelProvider.getOrCreate(document.uri, { metadata: this.metadata });
         }
         if (!kernel) {
             throw new Error('Unable to create a Kernel to run cell');

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

@@ -9,7 +9,6 @@ import { Uri } from 'vscode';
 import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../../common/application/types';
 import { traceInfo, traceWarning } from '../../../common/logger';
 import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types';
-import { IInterpreterService } from '../../../interpreter/contracts';
 import { IDataScienceErrorHandler, INotebookEditorProvider, INotebookProvider } from '../../types';
 import { Kernel } from './kernel';
 import { KernelSelector } from './kernelSelector';
@@ -24,7 +23,6 @@ export class KernelProvider implements IKernelProvider {
         @inject(INotebookProvider) private notebookProvider: INotebookProvider,
         @inject(IConfigurationService) private configService: IConfigurationService,
         @inject(ICommandManager) private readonly commandManager: ICommandManager,
-        @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
         @inject(IDataScienceErrorHandler) private readonly errorHandler: IDataScienceErrorHandler,
         @inject(INotebookEditorProvider) private readonly editorProvider: INotebookEditorProvider,
         @inject(KernelSelector) private readonly kernelSelectionUsage: IKernelSelectionUsage,
@@ -50,7 +48,6 @@ export class KernelProvider implements IKernelProvider {
             this.disposables,
             waitForIdleTimeout,
             this.commandManager,
-            this.interpreterService,
             this.errorHandler,
             this.editorProvider,
             this,

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

@@ -3,7 +3,7 @@
 
 'use strict';
 
-import type { Session } from '@jupyterlab/services';
+import type { KernelMessage, Session } from '@jupyterlab/services';
 import type { Observable } from 'rxjs/Observable';
 import type { CancellationToken, Event, QuickPickItem, Uri } from 'vscode';
 import { NotebookCell, NotebookDocument } from '../../../../../types/vscode-proposed';
@@ -130,6 +130,11 @@ export interface IKernel extends IAsyncDisposable {
     readonly onRestarted: Event<void>;
     readonly status: ServerStatus;
     readonly disposed: boolean;
+    /**
+     * Kernel information, used to save in ipynb in the metadata.
+     * Crucial for non-python notebooks, else we save the incorrect information.
+     */
+    readonly info?: KernelMessage.IInfoReplyMsg['content'];
     readonly kernelSocket: Observable<KernelSocketInformation | undefined>;
     start(): Promise<void>;
     interrupt(): Promise<InterruptResult>;

src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts

@@ -205,6 +205,16 @@ export class GuestJupyterNotebook
         }
     }
 
+    public async requestKernelInfo(): Promise<KernelMessage.IInfoReplyMsg> {
+        // This is a special case. Ask the shared server
+        const service = await this.waitForService();
+        if (service) {
+            return service.request(LiveShareCommands.getSysInfo, []);
+        } else {
+            throw new Error('No LiveShare service to get KernelInfo');
+        }
+    }
+
     public async getCompletion(
         _cellCode: string,
         _offsetInCode: number,

src/client/datascience/notebook/helpers/helpers.ts

@@ -27,6 +27,8 @@ import { JupyterNotebookView } from '../constants';
 // tslint:disable-next-line: no-var-requires no-require-imports
 const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
 // tslint:disable-next-line: no-require-imports
+import { KernelMessage } from '@jupyterlab/services';
+// tslint:disable-next-line: no-require-imports
 import cloneDeep = require('lodash/cloneDeep');
 import { isUntitledFile } from '../../../common/utils/misc';
 import { KernelConnectionMetadata } from '../../jupyter/kernels/types';
@@ -54,7 +56,10 @@ export function isJupyterNotebook(option: NotebookDocument | string) {
     }
 }
 
-const kernelInformationForNotebooks = new WeakMap<NotebookDocument, KernelConnectionMetadata | undefined>();
+const kernelInformationForNotebooks = new WeakMap<
+    NotebookDocument,
+    { metadata?: KernelConnectionMetadata | undefined; kernelInfo?: KernelMessage.IInfoReplyMsg['content'] }
+>();
 
 export function getNotebookMetadata(document: NotebookDocument): nbformat.INotebookMetadata | undefined {
     // tslint:disable-next-line: no-any
@@ -70,8 +75,9 @@ export function getNotebookMetadata(document: NotebookDocument): nbformat.INoteb
         notebookContent = { ...content, metadata: { ...metadata, language_info } } as any;
     }
     notebookContent = cloneDeep(notebookContent);
-    if (kernelInformationForNotebooks.has(document)) {
-        updateNotebookMetadata(notebookContent.metadata, kernelInformationForNotebooks.get(document));
+    const data = kernelInformationForNotebooks.get(document);
+    if (data && data.metadata) {
+        updateNotebookMetadata(notebookContent.metadata, data.metadata, data.kernelInfo);
     }
 
     return notebookContent.metadata;
@@ -88,7 +94,20 @@ export function updateKernelInNotebookMetadata(
     document: NotebookDocument,
     kernelConnection: KernelConnectionMetadata | undefined
 ) {
-    kernelInformationForNotebooks.set(document, kernelConnection);
+    const data = { ...(kernelInformationForNotebooks.get(document) || {}) };
+    data.metadata = kernelConnection;
+    kernelInformationForNotebooks.set(document, data);
+}
+export function updateKernelInfoInNotebookMetadata(
+    document: NotebookDocument,
+    kernelInfo: KernelMessage.IInfoReplyMsg['content']
+) {
+    if (kernelInformationForNotebooks.get(document)?.kernelInfo === kernelInfo) {
+        return;
+    }
+    const data = { ...(kernelInformationForNotebooks.get(document) || {}) };
+    data.kernelInfo = kernelInfo;
+    kernelInformationForNotebooks.set(document, data);
 }
 /**
  * Converts a NotebookModel into VSCode friendly format.