Fix kernel and server name missing in certain situations (microsoft#1…

…3974) * Fix kernel name and server name * Fixup server name for remote situations * Add some functional tests * Add news entry
karthiknadig · Sep 17, 2020 · 89b47a0 · 89b47a0
1 parent a7c3b7f
commit 89b47a0
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 87 deletions.
diff --git a/news/2 Fixes/13955.md b/news/2 Fixes/13955.md
@@ -0,0 +1 @@
+Make sure server name and kernel name show up when connecting.
diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts
@@ -47,7 +47,7 @@ import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
 import { generateCellRangesFromDocument } from '../cellFactory';
 import { CellMatcher } from '../cellMatcher';
 import { addToUriList, translateKernelLanguageToMonaco } from '../common';
-import { Commands, Identifiers, Telemetry } from '../constants';
+import { Commands, Identifiers, Settings, Telemetry } from '../constants';
 import { ColumnWarningSize, IDataViewerFactory } from '../data-viewing/types';
 import {
     IAddedSysInfo,
@@ -865,9 +865,54 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
             } catch (e) {
                 this.errorHandler.handleError(e).ignoreErrors();
             }
+        } else {
+            // Just send a kernel update so it shows something
+            this.postMessage(InteractiveWindowMessages.UpdateKernel, {
+                jupyterServerStatus: ServerStatus.NotStarted,
+                serverName: await this.getServerDisplayName(undefined),
+                kernelName: '',
+                language: PYTHON_LANGUAGE
+            }).ignoreErrors();
         }
     }
 
+    protected async getServerDisplayName(serverConnection: INotebookProviderConnection | undefined): Promise<string> {
+        // If we don't have a server connection, make one if remote. We need the remote connection in order
+        // to compute the display name. However only do this if the user is allowing auto start.
+        if (
+            !serverConnection &&
+            this.configService.getSettings(this.owningResource).datascience.jupyterServerURI !==
+                Settings.JupyterServerLocalLaunch &&
+            !this.configService.getSettings(this.owningResource).datascience.disableJupyterAutoStart
+        ) {
+            serverConnection = await this.notebookProvider.connect({ disableUI: true });
+        }
+
+        let displayName =
+            serverConnection?.displayName ||
+            (!serverConnection?.localLaunch ? serverConnection?.url : undefined) ||
+            (this.configService.getSettings().datascience.jupyterServerURI === Settings.JupyterServerLocalLaunch
+                ? localize.DataScience.localJupyterServer()
+                : localize.DataScience.serverNotStarted());
+
+        if (serverConnection) {
+            // Determine the connection URI of the connected server to display
+            if (serverConnection.localLaunch) {
+                displayName = localize.DataScience.localJupyterServer();
+            } else {
+                // Log this remote URI into our MRU list
+                addToUriList(
+                    this.globalStorage,
+                    !isNil(serverConnection.url) ? serverConnection.url : serverConnection.displayName,
+                    Date.now(),
+                    serverConnection.displayName
+                );
+            }
+        }
+
+        return displayName;
+    }
+
     private combineData(
         oldData: nbformat.ICodeCell | nbformat.IRawCell | nbformat.IMarkdownCell | undefined,
         cell: ICell
@@ -1154,36 +1199,15 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
         return notebook;
     }
 
-    private getServerUri(serverConnection: INotebookProviderConnection | undefined): string {
-        let localizedUri = '';
-
-        if (serverConnection) {
-            // Determine the connection URI of the connected server to display
-            if (serverConnection.localLaunch) {
-                localizedUri = localize.DataScience.localJupyterServer();
-            } else {
-                // Log this remote URI into our MRU list
-                addToUriList(
-                    this.globalStorage,
-                    !isNil(serverConnection.url) ? serverConnection.url : serverConnection.displayName,
-                    Date.now(),
-                    serverConnection.displayName
-                );
-            }
-        }
-
-        return localizedUri;
-    }
-
     private async listenToNotebookEvents(notebook: INotebook): Promise<void> {
         const statusChangeHandler = async (status: ServerStatus) => {
             const connectionMetadata = notebook.getKernelConnection();
             const name = getDisplayNameOrNameOfKernelConnection(connectionMetadata);
 
             await this.postMessage(InteractiveWindowMessages.UpdateKernel, {
                 jupyterServerStatus: status,
-                localizedUri: this.getServerUri(notebook.connection),
-                displayName: name,
+                serverName: await this.getServerDisplayName(notebook.connection),
+                kernelName: name,
                 language: translateKernelLanguageToMonaco(
                     getKernelConnectionLanguage(connectionMetadata) || PYTHON_LANGUAGE
                 )
@@ -1205,8 +1229,8 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
             // While waiting make the notebook look busy
             this.postMessage(InteractiveWindowMessages.UpdateKernel, {
                 jupyterServerStatus: ServerStatus.Busy,
-                localizedUri: this.getServerUri(serverConnection),
-                displayName: '',
+                serverName: await this.getServerDisplayName(serverConnection),
+                kernelName: '',
                 language: PYTHON_LANGUAGE
             }).ignoreErrors();
 
@@ -1234,8 +1258,8 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
             // No notebook, send update to UI anyway
             this.postMessage(InteractiveWindowMessages.UpdateKernel, {
                 jupyterServerStatus: ServerStatus.NotStarted,
-                localizedUri: '',
-                displayName: getDisplayNameOrNameOfKernelConnection(data.kernelConnection),
+                serverName: await this.getServerDisplayName(undefined),
+                kernelName: getDisplayNameOrNameOfKernelConnection(data.kernelConnection),
                 language: translateKernelLanguageToMonaco(
                     getKernelConnectionLanguage(data.kernelConnection) || PYTHON_LANGUAGE
                 )

diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts
@@ -751,8 +751,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
             if (!this.notebook && metadata?.kernelspec) {
                 this.postMessage(InteractiveWindowMessages.UpdateKernel, {
                     jupyterServerStatus: ServerStatus.NotStarted,
-                    localizedUri: '',
-                    displayName: metadata.kernelspec.display_name ?? metadata.kernelspec.name,
+                    serverName: await this.getServerDisplayName(undefined),
+                    kernelName: metadata.kernelspec.display_name ?? metadata.kernelspec.name,
                     language: translateKernelLanguageToMonaco(
                         (metadata.kernelspec.language as string) ?? PYTHON_LANGUAGE
                     )

diff --git a/src/client/datascience/jupyter/kernels/helpers.ts b/src/client/datascience/jupyter/kernels/helpers.ts
@@ -64,7 +64,12 @@ export function getDisplayNameOrNameOfKernelConnection(
         kernelConnection.kind === 'connectToLiveKernel'
             ? kernelConnection.kernelModel.name
             : kernelConnection.kernelSpec?.name;
-    return displayName || name || defaultValue;
+
+    const interpeterName =
+        kernelConnection.kind === 'startUsingPythonInterpreter' ? kernelConnection.interpreter.displayName : undefined;
+
+    const defaultKernelName = kernelConnection.kind === 'startUsingDefaultKernel' ? 'Python 3' : undefined;
+    return displayName || name || interpeterName || defaultKernelName || defaultValue;
 }
 
 export function getNameOfKernelConnection(

diff --git a/src/datascience-ui/history-react/interactivePanel.tsx b/src/datascience-ui/history-react/interactivePanel.tsx
@@ -223,7 +223,7 @@ ${buildSettingsCss(this.props.settings)}`}</style>
     }
 
     private renderKernelSelection() {
-        if (this.props.kernel.localizedUri === getLocString('DataScience.localJupyterServer', 'local')) {
+        if (this.props.kernel.serverName === getLocString('DataScience.localJupyterServer', 'local')) {
             return;
         }
 
@@ -235,7 +235,6 @@ ${buildSettingsCss(this.props.settings)}`}</style>
                 selectServer={this.props.selectServer}
                 selectKernel={this.props.selectKernel}
                 shouldShowTrustMessage={false}
-                settings={this.props.settings}
             />
         );
     }

diff --git a/src/datascience-ui/interactive-common/jupyterInfo.tsx b/src/datascience-ui/interactive-common/jupyterInfo.tsx
@@ -1,9 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 'use strict';
-import { isEmpty, isNil } from 'lodash';
 import * as React from 'react';
-import { IDataScienceExtraSettings } from '../../client/datascience/types';
 import { Image, ImageName } from '../react-common/image';
 import { getLocString } from '../react-common/locReactSide';
 import { IFont, IServerState, ServerStatus } from './mainState';
@@ -16,7 +14,6 @@ export interface IJupyterInfoProps {
     kernel: IServerState;
     isNotebookTrusted?: boolean;
     shouldShowTrustMessage: boolean;
-    settings?: IDataScienceExtraSettings | undefined;
     selectServer(): void;
     launchNotebookTrustPrompt?(): void; // Native editor-specific
     selectKernel(): void;
@@ -37,17 +34,10 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
     }
 
     public render() {
-        let jupyterServerDisplayName: string = this.props.kernel.localizedUri;
-        if (!isNil(this.props.settings) && isEmpty(jupyterServerDisplayName)) {
-            const jupyterServerUriSetting: string = this.props.settings.jupyterServerURI;
-            if (!isEmpty(jupyterServerUriSetting) && this.isUriOfComputeInstance(jupyterServerUriSetting)) {
-                jupyterServerDisplayName = this.getComputeInstanceNameFromId(jupyterServerUriSetting);
-            }
-        }
-
+        const jupyterServerDisplayName: string = this.props.kernel.serverName;
         const serverTextSize =
             getLocString('DataScience.jupyterServer', 'Jupyter Server').length + jupyterServerDisplayName.length + 4; // plus 4 for the icon
-        const displayNameTextSize = this.props.kernel.displayName.length + this.props.kernel.jupyterServerStatus.length;
+        const displayNameTextSize = this.props.kernel.kernelName.length + this.props.kernel.jupyterServerStatus.length;
         const dynamicFont: React.CSSProperties = {
             fontSize: 'var(--vscode-font-size)', // Use the same font and size as the menu
             fontFamily: 'var(--vscode-font-family)',
@@ -98,11 +88,11 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
                     role="button"
                     aria-disabled={ariaDisabled}
                 >
-                    {this.props.kernel.displayName}: {this.props.kernel.jupyterServerStatus}
+                    {this.props.kernel.kernelName}: {this.props.kernel.jupyterServerStatus}
                 </div>
             );
         } else {
-            const displayName = this.props.kernel.displayName ?? getLocString('DataScience.noKernel', 'No Kernel');
+            const displayName = this.props.kernel.kernelName ?? getLocString('DataScience.noKernel', 'No Kernel');
             return (
                 <div className="kernel-status-section kernel-status-status" style={displayNameTextWidth} role="button">
                     {displayName}: {this.props.kernel.jupyterServerStatus}
@@ -138,30 +128,6 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
             : getLocString('DataScience.connected', 'Connected');
     }
 
-    private isUriOfComputeInstance(uri: string): boolean {
-        try {
-            const parsedUrl: URL = new URL(uri);
-            return parsedUrl.searchParams.get('id') === 'azureml_compute_instances';
-        } catch (e) {
-            return false;
-        }
-    }
-
-    private getComputeInstanceNameFromId(id: string | undefined): string {
-        if (isNil(id)) {
-            return '';
-        }
-
-        const res: string[] | null = id.match(
-            /\/providers\/Microsoft.MachineLearningServices\/workspaces\/[^\/]+\/computes\/([^\/]+)(\/)?/
-        );
-        if (isNil(res) || res.length < 2) {
-            return '';
-        }
-
-        return res[1];
-    }
-
     private selectServer(): void {
         this.props.selectServer();
     }

diff --git a/src/datascience-ui/interactive-common/mainState.ts b/src/datascience-ui/interactive-common/mainState.ts
@@ -134,8 +134,8 @@ export interface IFont {
 
 export interface IServerState {
     jupyterServerStatus: ServerStatus;
-    localizedUri: string;
-    displayName: string;
+    serverName: string;
+    kernelName: string;
     language: string;
 }
 
@@ -192,8 +192,8 @@ export function generateTestState(filePath: string = '', editable: boolean = fal
         loaded: false,
         testMode: true,
         kernel: {
-            localizedUri: 'No Kernel',
-            displayName: 'Python',
+            serverName: '',
+            kernelName: 'Python',
             jupyterServerStatus: ServerStatus.NotStarted,
             language: PYTHON_LANGUAGE
         },

diff --git a/src/datascience-ui/interactive-common/redux/reducers/kernel.ts b/src/datascience-ui/interactive-common/redux/reducers/kernel.ts
@@ -40,9 +40,9 @@ export namespace Kernel {
             return {
                 ...arg.prevState,
                 kernel: {
-                    localizedUri: arg.payload.data.localizedUri,
+                    serverName: arg.payload.data.serverName,
                     jupyterServerStatus: arg.payload.data.jupyterServerStatus,
-                    displayName: arg.payload.data.displayName,
+                    kernelName: arg.payload.data.kernelName,
                     language: arg.payload.data.language
                 }
             };

diff --git a/src/datascience-ui/interactive-common/redux/store.ts b/src/datascience-ui/interactive-common/redux/store.ts
@@ -68,8 +68,8 @@ function generateDefaultState(
             monacoReady: testMode, // When testing, monaco starts out ready
             loaded: false,
             kernel: {
-                displayName: getLocString('DataScience.noKernel', 'No Kernel'),
-                localizedUri: getLocString('DataScience.serverNotStarted', 'Not Started'),
+                kernelName: getLocString('DataScience.noKernel', 'No Kernel'),
+                serverName: getLocString('DataScience.serverNotStarted', 'Not Started'),
                 jupyterServerStatus: ServerStatus.NotStarted,
                 language: PYTHON_LANGUAGE
             },

diff --git a/src/datascience-ui/native-editor/toolbar.tsx b/src/datascience-ui/native-editor/toolbar.tsx
@@ -268,7 +268,6 @@ export class Toolbar extends React.PureComponent<INativeEditorToolbarProps> {
                         shouldShowTrustMessage={true}
                         isNotebookTrusted={this.props.isNotebookTrusted}
                         launchNotebookTrustPrompt={launchNotebookTrustPrompt}
-                        settings={this.props.settings}
                     />
                 </div>
                 <div className="toolbar-divider" />

diff --git a/src/test/datascience/interactivePanel.functional.test.tsx b/src/test/datascience/interactivePanel.functional.test.tsx
@@ -76,9 +76,9 @@ suite('DataScience Interactive Panel', () => {
             interruptKernel: noopAny,
             isAtBottom: false,
             kernel: {
-                displayName: '',
+                kernelName: '',
                 jupyterServerStatus: ServerStatus.Busy,
-                localizedUri: '',
+                serverName: '',
                 language: PYTHON_LANGUAGE
             },
             knownDark: false,

diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx
@@ -83,7 +83,8 @@ import {
     typeCode,
     verifyCellIndex,
     verifyCellSource,
-    verifyHtmlOnCell
+    verifyHtmlOnCell,
+    verifyServerStatus
 } from './testHelpers';
 import { ITestNativeEditorProvider } from './testNativeEditorProvider';
 
@@ -713,6 +714,8 @@ df.head()`;
 
                         // Make sure it has a server
                         assert.ok(editor.editor.notebook, 'Notebook did not start with a server');
+                        // Make sure it does have a name though
+                        verifyServerStatus(editor.mount.wrapper, 'local');
                     } else {
                         context.skip();
                     }
@@ -721,6 +724,7 @@ df.head()`;
                 runMountedTest('Server load skipped', async (context) => {
                     if (ioc.mockJupyter) {
                         ioc.getSettings().datascience.disableJupyterAutoStart = true;
+                        ioc.getSettings().datascience.jupyterServerURI = 'https://remotetest';
                         await ioc.activate();
 
                         // Create an editor so something is listening to messages
@@ -731,6 +735,9 @@ df.head()`;
 
                         // Make sure it does not have a server
                         assert.notOk(editor.editor.notebook, 'Notebook should not start with a server');
+
+                        // Make sure it does have a name though
+                        verifyServerStatus(editor.mount.wrapper, 'Not Started');
                     } else {
                         context.skip();
                     }

diff --git a/src/test/datascience/nativeEditor.toolbar.functional.test.tsx b/src/test/datascience/nativeEditor.toolbar.functional.test.tsx
@@ -45,9 +45,9 @@ suite('DataScience Native Toolbar', () => {
             font: { family: '', size: 1 },
             interruptKernel: sinon.stub(),
             kernel: {
-                displayName: '',
+                kernelName: '',
                 jupyterServerStatus: ServerStatus.Busy,
-                localizedUri: '',
+                serverName: '',
                 language: PYTHON_LANGUAGE
             },
             restartKernel: sinon.stub(),
@@ -245,9 +245,9 @@ suite('DataScience Native Toolbar', () => {
                 font: { family: '', size: 1 },
                 interruptKernel: sinon.stub(),
                 kernel: {
-                    displayName: '',
+                    kernelName: '',
                     jupyterServerStatus: ServerStatus.Busy,
-                    localizedUri: '',
+                    serverName: '',
                     language: PYTHON_LANGUAGE
                 },
                 restartKernel: sinon.stub(),

diff --git a/src/test/datascience/testHelpers.tsx b/src/test/datascience/testHelpers.tsx
@@ -221,6 +221,15 @@ export function verifyCellSource(
     assert.deepStrictEqual(inst.state.model?.getValue(), source, 'Source does not match on cell');
 }
 
+export function verifyServerStatus(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, statusText: string) {
+    wrapper.update();
+
+    const foundResult = wrapper.find('div.kernel-status-server');
+    assert.ok(foundResult.length >= 1, "Didn't find server status");
+    const html = foundResult.html();
+    assert.ok(html.includes(statusText), `${statusText} not found in server status`);
+}
+
 export function verifyHtmlOnCell(
     wrapper: ReactWrapper<any, Readonly<{}>, React.Component>,
     cellType: 'NativeCell' | 'InteractiveCell',