Skip to content

Commit 89b47a0

Browse files
rchiodorchiodo
authored
Fix kernel and server name missing in certain situations (#13974)
* Fix kernel name and server name * Fixup server name for remote situations * Add some functional tests * Add news entry
1 parent a7c3b7f commit 89b47a0

File tree

14 files changed

+97
-87
lines changed

14 files changed

+97
-87
lines changed

‎news/2 Fixes/13955.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Make sure server name and kernel name show up when connecting.

‎src/client/datascience/interactive-common/interactiveBase.ts

Lines changed: 52 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
4747
import { generateCellRangesFromDocument } from '../cellFactory';
4848
import { CellMatcher } from '../cellMatcher';
4949
import { addToUriList, translateKernelLanguageToMonaco } from '../common';
50-
import { Commands, Identifiers, Telemetry } from '../constants';
50+
import { Commands, Identifiers, Settings, Telemetry } from '../constants';
5151
import { ColumnWarningSize, IDataViewerFactory } from '../data-viewing/types';
5252
import {
5353
IAddedSysInfo,
@@ -865,9 +865,54 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
865865
} catch (e) {
866866
this.errorHandler.handleError(e).ignoreErrors();
867867
}
868+
} else {
869+
// Just send a kernel update so it shows something
870+
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
871+
jupyterServerStatus: ServerStatus.NotStarted,
872+
serverName: await this.getServerDisplayName(undefined),
873+
kernelName: '',
874+
language: PYTHON_LANGUAGE
875+
}).ignoreErrors();
868876
}
869877
}
870878

879+
protected async getServerDisplayName(serverConnection: INotebookProviderConnection | undefined): Promise<string> {
880+
// If we don't have a server connection, make one if remote. We need the remote connection in order
881+
// to compute the display name. However only do this if the user is allowing auto start.
882+
if (
883+
!serverConnection &&
884+
this.configService.getSettings(this.owningResource).datascience.jupyterServerURI !==
885+
Settings.JupyterServerLocalLaunch &&
886+
!this.configService.getSettings(this.owningResource).datascience.disableJupyterAutoStart
887+
) {
888+
serverConnection = await this.notebookProvider.connect({ disableUI: true });
889+
}
890+
891+
let displayName =
892+
serverConnection?.displayName ||
893+
(!serverConnection?.localLaunch ? serverConnection?.url : undefined) ||
894+
(this.configService.getSettings().datascience.jupyterServerURI === Settings.JupyterServerLocalLaunch
895+
? localize.DataScience.localJupyterServer()
896+
: localize.DataScience.serverNotStarted());
897+
898+
if (serverConnection) {
899+
// Determine the connection URI of the connected server to display
900+
if (serverConnection.localLaunch) {
901+
displayName = localize.DataScience.localJupyterServer();
902+
} else {
903+
// Log this remote URI into our MRU list
904+
addToUriList(
905+
this.globalStorage,
906+
!isNil(serverConnection.url) ? serverConnection.url : serverConnection.displayName,
907+
Date.now(),
908+
serverConnection.displayName
909+
);
910+
}
911+
}
912+
913+
return displayName;
914+
}
915+
871916
private combineData(
872917
oldData: nbformat.ICodeCell | nbformat.IRawCell | nbformat.IMarkdownCell | undefined,
873918
cell: ICell
@@ -1154,36 +1199,15 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
11541199
return notebook;
11551200
}
11561201

1157-
private getServerUri(serverConnection: INotebookProviderConnection | undefined): string {
1158-
let localizedUri = '';
1159-
1160-
if (serverConnection) {
1161-
// Determine the connection URI of the connected server to display
1162-
if (serverConnection.localLaunch) {
1163-
localizedUri = localize.DataScience.localJupyterServer();
1164-
} else {
1165-
// Log this remote URI into our MRU list
1166-
addToUriList(
1167-
this.globalStorage,
1168-
!isNil(serverConnection.url) ? serverConnection.url : serverConnection.displayName,
1169-
Date.now(),
1170-
serverConnection.displayName
1171-
);
1172-
}
1173-
}
1174-
1175-
return localizedUri;
1176-
}
1177-
11781202
private async listenToNotebookEvents(notebook: INotebook): Promise<void> {
11791203
const statusChangeHandler = async (status: ServerStatus) => {
11801204
const connectionMetadata = notebook.getKernelConnection();
11811205
const name = getDisplayNameOrNameOfKernelConnection(connectionMetadata);
11821206

11831207
await this.postMessage(InteractiveWindowMessages.UpdateKernel, {
11841208
jupyterServerStatus: status,
1185-
localizedUri: this.getServerUri(notebook.connection),
1186-
displayName: name,
1209+
serverName: await this.getServerDisplayName(notebook.connection),
1210+
kernelName: name,
11871211
language: translateKernelLanguageToMonaco(
11881212
getKernelConnectionLanguage(connectionMetadata) || PYTHON_LANGUAGE
11891213
)
@@ -1205,8 +1229,8 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
12051229
// While waiting make the notebook look busy
12061230
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
12071231
jupyterServerStatus: ServerStatus.Busy,
1208-
localizedUri: this.getServerUri(serverConnection),
1209-
displayName: '',
1232+
serverName: await this.getServerDisplayName(serverConnection),
1233+
kernelName: '',
12101234
language: PYTHON_LANGUAGE
12111235
}).ignoreErrors();
12121236

@@ -1234,8 +1258,8 @@ export abstract class InteractiveBase extends WebviewPanelHost<IInteractiveWindo
12341258
// No notebook, send update to UI anyway
12351259
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
12361260
jupyterServerStatus: ServerStatus.NotStarted,
1237-
localizedUri: '',
1238-
displayName: getDisplayNameOrNameOfKernelConnection(data.kernelConnection),
1261+
serverName: await this.getServerDisplayName(undefined),
1262+
kernelName: getDisplayNameOrNameOfKernelConnection(data.kernelConnection),
12391263
language: translateKernelLanguageToMonaco(
12401264
getKernelConnectionLanguage(data.kernelConnection) || PYTHON_LANGUAGE
12411265
)

‎src/client/datascience/interactive-ipynb/nativeEditor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -751,8 +751,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
751751
if (!this.notebook && metadata?.kernelspec) {
752752
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
753753
jupyterServerStatus: ServerStatus.NotStarted,
754-
localizedUri: '',
755-
displayName: metadata.kernelspec.display_name ?? metadata.kernelspec.name,
754+
serverName: await this.getServerDisplayName(undefined),
755+
kernelName: metadata.kernelspec.display_name ?? metadata.kernelspec.name,
756756
language: translateKernelLanguageToMonaco(
757757
(metadata.kernelspec.language as string) ?? PYTHON_LANGUAGE
758758
)

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,12 @@ export function getDisplayNameOrNameOfKernelConnection(
6464
kernelConnection.kind === 'connectToLiveKernel'
6565
? kernelConnection.kernelModel.name
6666
: kernelConnection.kernelSpec?.name;
67-
return displayName || name || defaultValue;
67+
68+
const interpeterName =
69+
kernelConnection.kind === 'startUsingPythonInterpreter' ? kernelConnection.interpreter.displayName : undefined;
70+
71+
const defaultKernelName = kernelConnection.kind === 'startUsingDefaultKernel' ? 'Python 3' : undefined;
72+
return displayName || name || interpeterName || defaultKernelName || defaultValue;
6873
}
6974

7075
export function getNameOfKernelConnection(

‎src/datascience-ui/history-react/interactivePanel.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ ${buildSettingsCss(this.props.settings)}`}</style>
223223
}
224224

225225
private renderKernelSelection() {
226-
if (this.props.kernel.localizedUri === getLocString('DataScience.localJupyterServer', 'local')) {
226+
if (this.props.kernel.serverName === getLocString('DataScience.localJupyterServer', 'local')) {
227227
return;
228228
}
229229

@@ -235,7 +235,6 @@ ${buildSettingsCss(this.props.settings)}`}</style>
235235
selectServer={this.props.selectServer}
236236
selectKernel={this.props.selectKernel}
237237
shouldShowTrustMessage={false}
238-
settings={this.props.settings}
239238
/>
240239
);
241240
}

‎src/datascience-ui/interactive-common/jupyterInfo.tsx

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
'use strict';
4-
import { isEmpty, isNil } from 'lodash';
54
import * as React from 'react';
6-
import { IDataScienceExtraSettings } from '../../client/datascience/types';
75
import { Image, ImageName } from '../react-common/image';
86
import { getLocString } from '../react-common/locReactSide';
97
import { IFont, IServerState, ServerStatus } from './mainState';
@@ -16,7 +14,6 @@ export interface IJupyterInfoProps {
1614
kernel: IServerState;
1715
isNotebookTrusted?: boolean;
1816
shouldShowTrustMessage: boolean;
19-
settings?: IDataScienceExtraSettings | undefined;
2017
selectServer(): void;
2118
launchNotebookTrustPrompt?(): void; // Native editor-specific
2219
selectKernel(): void;
@@ -37,17 +34,10 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
3734
}
3835

3936
public render() {
40-
let jupyterServerDisplayName: string = this.props.kernel.localizedUri;
41-
if (!isNil(this.props.settings) && isEmpty(jupyterServerDisplayName)) {
42-
const jupyterServerUriSetting: string = this.props.settings.jupyterServerURI;
43-
if (!isEmpty(jupyterServerUriSetting) && this.isUriOfComputeInstance(jupyterServerUriSetting)) {
44-
jupyterServerDisplayName = this.getComputeInstanceNameFromId(jupyterServerUriSetting);
45-
}
46-
}
47-
37+
const jupyterServerDisplayName: string = this.props.kernel.serverName;
4838
const serverTextSize =
4939
getLocString('DataScience.jupyterServer', 'Jupyter Server').length + jupyterServerDisplayName.length + 4; // plus 4 for the icon
50-
const displayNameTextSize = this.props.kernel.displayName.length + this.props.kernel.jupyterServerStatus.length;
40+
const displayNameTextSize = this.props.kernel.kernelName.length + this.props.kernel.jupyterServerStatus.length;
5141
const dynamicFont: React.CSSProperties = {
5242
fontSize: 'var(--vscode-font-size)', // Use the same font and size as the menu
5343
fontFamily: 'var(--vscode-font-family)',
@@ -98,11 +88,11 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
9888
role="button"
9989
aria-disabled={ariaDisabled}
10090
>
101-
{this.props.kernel.displayName}: {this.props.kernel.jupyterServerStatus}
91+
{this.props.kernel.kernelName}: {this.props.kernel.jupyterServerStatus}
10292
</div>
10393
);
10494
} else {
105-
const displayName = this.props.kernel.displayName ?? getLocString('DataScience.noKernel', 'No Kernel');
95+
const displayName = this.props.kernel.kernelName ?? getLocString('DataScience.noKernel', 'No Kernel');
10696
return (
10797
<div className="kernel-status-section kernel-status-status" style={displayNameTextWidth} role="button">
10898
{displayName}: {this.props.kernel.jupyterServerStatus}
@@ -138,30 +128,6 @@ export class JupyterInfo extends React.Component<IJupyterInfoProps> {
138128
: getLocString('DataScience.connected', 'Connected');
139129
}
140130

141-
private isUriOfComputeInstance(uri: string): boolean {
142-
try {
143-
const parsedUrl: URL = new URL(uri);
144-
return parsedUrl.searchParams.get('id') === 'azureml_compute_instances';
145-
} catch (e) {
146-
return false;
147-
}
148-
}
149-
150-
private getComputeInstanceNameFromId(id: string | undefined): string {
151-
if (isNil(id)) {
152-
return '';
153-
}
154-
155-
const res: string[] | null = id.match(
156-
/\/providers\/Microsoft.MachineLearningServices\/workspaces\/[^\/]+\/computes\/([^\/]+)(\/)?/
157-
);
158-
if (isNil(res) || res.length < 2) {
159-
return '';
160-
}
161-
162-
return res[1];
163-
}
164-
165131
private selectServer(): void {
166132
this.props.selectServer();
167133
}

‎src/datascience-ui/interactive-common/mainState.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ export interface IFont {
134134

135135
export interface IServerState {
136136
jupyterServerStatus: ServerStatus;
137-
localizedUri: string;
138-
displayName: string;
137+
serverName: string;
138+
kernelName: string;
139139
language: string;
140140
}
141141

@@ -192,8 +192,8 @@ export function generateTestState(filePath: string = '', editable: boolean = fal
192192
loaded: false,
193193
testMode: true,
194194
kernel: {
195-
localizedUri: 'No Kernel',
196-
displayName: 'Python',
195+
serverName: '',
196+
kernelName: 'Python',
197197
jupyterServerStatus: ServerStatus.NotStarted,
198198
language: PYTHON_LANGUAGE
199199
},

‎src/datascience-ui/interactive-common/redux/reducers/kernel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ export namespace Kernel {
4040
return {
4141
...arg.prevState,
4242
kernel: {
43-
localizedUri: arg.payload.data.localizedUri,
43+
serverName: arg.payload.data.serverName,
4444
jupyterServerStatus: arg.payload.data.jupyterServerStatus,
45-
displayName: arg.payload.data.displayName,
45+
kernelName: arg.payload.data.kernelName,
4646
language: arg.payload.data.language
4747
}
4848
};

‎src/datascience-ui/interactive-common/redux/store.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ function generateDefaultState(
6868
monacoReady: testMode, // When testing, monaco starts out ready
6969
loaded: false,
7070
kernel: {
71-
displayName: getLocString('DataScience.noKernel', 'No Kernel'),
72-
localizedUri: getLocString('DataScience.serverNotStarted', 'Not Started'),
71+
kernelName: getLocString('DataScience.noKernel', 'No Kernel'),
72+
serverName: getLocString('DataScience.serverNotStarted', 'Not Started'),
7373
jupyterServerStatus: ServerStatus.NotStarted,
7474
language: PYTHON_LANGUAGE
7575
},

‎src/datascience-ui/native-editor/toolbar.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,6 @@ export class Toolbar extends React.PureComponent<INativeEditorToolbarProps> {
268268
shouldShowTrustMessage={true}
269269
isNotebookTrusted={this.props.isNotebookTrusted}
270270
launchNotebookTrustPrompt={launchNotebookTrustPrompt}
271-
settings={this.props.settings}
272271
/>
273272
</div>
274273
<div className="toolbar-divider" />

‎src/test/datascience/interactivePanel.functional.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ suite('DataScience Interactive Panel', () => {
7676
interruptKernel: noopAny,
7777
isAtBottom: false,
7878
kernel: {
79-
displayName: '',
79+
kernelName: '',
8080
jupyterServerStatus: ServerStatus.Busy,
81-
localizedUri: '',
81+
serverName: '',
8282
language: PYTHON_LANGUAGE
8383
},
8484
knownDark: false,

‎src/test/datascience/nativeEditor.functional.test.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ import {
8383
typeCode,
8484
verifyCellIndex,
8585
verifyCellSource,
86-
verifyHtmlOnCell
86+
verifyHtmlOnCell,
87+
verifyServerStatus
8788
} from './testHelpers';
8889
import { ITestNativeEditorProvider } from './testNativeEditorProvider';
8990

@@ -713,6 +714,8 @@ df.head()`;
713714

714715
// Make sure it has a server
715716
assert.ok(editor.editor.notebook, 'Notebook did not start with a server');
717+
// Make sure it does have a name though
718+
verifyServerStatus(editor.mount.wrapper, 'local');
716719
} else {
717720
context.skip();
718721
}
@@ -721,6 +724,7 @@ df.head()`;
721724
runMountedTest('Server load skipped', async (context) => {
722725
if (ioc.mockJupyter) {
723726
ioc.getSettings().datascience.disableJupyterAutoStart = true;
727+
ioc.getSettings().datascience.jupyterServerURI = 'https://remotetest';
724728
await ioc.activate();
725729

726730
// Create an editor so something is listening to messages
@@ -731,6 +735,9 @@ df.head()`;
731735

732736
// Make sure it does not have a server
733737
assert.notOk(editor.editor.notebook, 'Notebook should not start with a server');
738+
739+
// Make sure it does have a name though
740+
verifyServerStatus(editor.mount.wrapper, 'Not Started');
734741
} else {
735742
context.skip();
736743
}

‎src/test/datascience/nativeEditor.toolbar.functional.test.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ suite('DataScience Native Toolbar', () => {
4545
font: { family: '', size: 1 },
4646
interruptKernel: sinon.stub(),
4747
kernel: {
48-
displayName: '',
48+
kernelName: '',
4949
jupyterServerStatus: ServerStatus.Busy,
50-
localizedUri: '',
50+
serverName: '',
5151
language: PYTHON_LANGUAGE
5252
},
5353
restartKernel: sinon.stub(),
@@ -245,9 +245,9 @@ suite('DataScience Native Toolbar', () => {
245245
font: { family: '', size: 1 },
246246
interruptKernel: sinon.stub(),
247247
kernel: {
248-
displayName: '',
248+
kernelName: '',
249249
jupyterServerStatus: ServerStatus.Busy,
250-
localizedUri: '',
250+
serverName: '',
251251
language: PYTHON_LANGUAGE
252252
},
253253
restartKernel: sinon.stub(),

‎src/test/datascience/testHelpers.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,15 @@ export function verifyCellSource(
221221
assert.deepStrictEqual(inst.state.model?.getValue(), source, 'Source does not match on cell');
222222
}
223223

224+
export function verifyServerStatus(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, statusText: string) {
225+
wrapper.update();
226+
227+
const foundResult = wrapper.find('div.kernel-status-server');
228+
assert.ok(foundResult.length >= 1, "Didn't find server status");
229+
const html = foundResult.html();
230+
assert.ok(html.includes(statusText), `${statusText} not found in server status`);
231+
}
232+
224233
export function verifyHtmlOnCell(
225234
wrapper: ReactWrapper<any, Readonly<{}>, React.Component>,
226235
cellType: 'NativeCell' | 'InteractiveCell',

0 commit comments

Comments
 (0)