Skip to content

Commit fedd350

Browse files
authored
Use KernelSelection instead of KernelSpecInterpreter (microsoft#13416)
For #13406 Ensure we use the same type across most of the code. Still haven't renamed kernelSelection to KernelConnectionMetadata (that is a separate PR)
1 parent f861aa7 commit fedd350

24 files changed

+398
-358
lines changed

src/client/common/application/commands.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { CancellationToken, Position, TextDocument, Uri } from 'vscode';
77
import { Commands as LSCommands } from '../../activation/commands';
88
import { Commands as DSCommands } from '../../datascience/constants';
9-
import { KernelSpecInterpreter } from '../../datascience/jupyter/kernels/kernelSelector';
9+
import { KernelConnectionMetadata } from '../../datascience/jupyter/kernels/types';
1010
import { INotebookModel, ISwitchKernelOptions } from '../../datascience/types';
1111
import { CommandSource } from '../../testing/common/constants';
1212
import { TestFunction, TestsToRun } from '../../testing/common/types';
@@ -192,7 +192,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
192192
[DSCommands.ExportToHTML]: [INotebookModel, string | undefined];
193193
[DSCommands.ExportToPDF]: [INotebookModel, string | undefined];
194194
[DSCommands.Export]: [Uri | INotebookModel, string | undefined];
195-
[DSCommands.SetJupyterKernel]: [KernelSpecInterpreter, Uri, undefined | Uri];
195+
[DSCommands.SetJupyterKernel]: [KernelConnectionMetadata, Uri, undefined | Uri];
196196
[DSCommands.SwitchJupyterKernel]: [ISwitchKernelOptions | undefined];
197197
[DSCommands.SelectJupyterCommandLine]: [undefined | Uri];
198198
[DSCommands.SaveNotebookNonCustomEditor]: [Uri];

src/client/common/utils/localize.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ export namespace DataScience {
932932
'# Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|  &nbsp|This notebook was generated by the Gather Extension. It requires version 2020.7.94776 (or newer) of the Python Extension, please update [here](https://command:python.datascience.latestExtension). The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)'
933933
);
934934
export const savePngTitle = localize('DataScience.savePngTitle', 'Save Image');
935-
export const fallbackToUseActiveInterpeterAsKernel = localize(
935+
export const fallbackToUseActiveInterpreterAsKernel = localize(
936936
'DataScience.fallbackToUseActiveInterpeterAsKernel',
937937
"Couldn't find kernel '{0}' that the notebook was created with. Using the current interpreter."
938938
);

src/client/datascience/commands/notebookCommands.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import { Uri } from 'vscode';
88
import { ICommandManager } from '../../common/application/types';
99
import { IDisposable } from '../../common/types';
1010
import { Commands } from '../constants';
11-
import { KernelSelector, KernelSpecInterpreter } from '../jupyter/kernels/kernelSelector';
11+
import { kernelConnectionMetadataHasKernelModel } from '../jupyter/kernels/helpers';
12+
import { KernelSelector } from '../jupyter/kernels/kernelSelector';
1213
import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher';
14+
import { KernelConnectionMetadata } from '../jupyter/kernels/types';
1315
import { IInteractiveWindowProvider, INotebookEditorProvider, INotebookProvider, ISwitchKernelOptions } from '../types';
1416

1517
@injectable()
@@ -69,9 +71,9 @@ export class NotebookCommands implements IDisposable {
6971
}
7072
}
7173

72-
private async setKernel(kernel: KernelSpecInterpreter, identity: Uri, resource: Uri | undefined) {
73-
const specOrModel = kernel?.kernelModel || kernel?.kernelSpec;
74-
if (kernel && specOrModel) {
74+
private async setKernel(kernel: KernelConnectionMetadata, identity: Uri, resource: Uri | undefined) {
75+
const specOrModel = kernelConnectionMetadataHasKernelModel(kernel) ? kernel.kernelModel : kernel.kernelSpec;
76+
if (specOrModel) {
7577
const notebook = await this.notebookProvider.getOrCreateNotebook({
7678
resource,
7779
identity,

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

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,13 @@ import {
7070
VariableExplorerStateKeys
7171
} from '../interactive-common/interactiveWindowTypes';
7272
import { JupyterInvalidKernelError } from '../jupyter/jupyterInvalidKernelError';
73+
import {
74+
kernelConnectionMetadataHasKernelModel,
75+
kernelConnectionMetadataHasKernelSpec
76+
} from '../jupyter/kernels/helpers';
7377
import { JupyterKernelPromiseFailedError } from '../jupyter/kernels/jupyterKernelPromiseFailedError';
74-
import { KernelSelector, KernelSpecInterpreter } from '../jupyter/kernels/kernelSelector';
75-
import { LiveKernelModel } from '../jupyter/kernels/types';
78+
import { KernelSelector } from '../jupyter/kernels/kernelSelector';
79+
import { KernelConnectionMetadata, LiveKernelModel } from '../jupyter/kernels/types';
7680
import { CssMessages, SharedMessages } from '../messages';
7781
import {
7882
CellState,
@@ -1144,7 +1148,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
11441148
serverConnection.type,
11451149
e.kernelSpec
11461150
);
1147-
if (newKernel?.kernelSpec) {
1151+
if (newKernel && kernelConnectionMetadataHasKernelSpec(newKernel) && newKernel.kernelSpec) {
11481152
this.commandManager.executeCommand(
11491153
Commands.SetJupyterKernel,
11501154
newKernel,
@@ -1224,15 +1228,20 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
12241228
this.postMessage(InteractiveWindowMessages.ForceVariableRefresh).ignoreErrors();
12251229
}
12261230

1227-
private async potentialKernelChanged(data: { identity: Uri; kernel: KernelSpecInterpreter }): Promise<void> {
1228-
const specOrModel = data.kernel.kernelModel || data.kernel.kernelSpec;
1231+
private async potentialKernelChanged(data: { identity: Uri; kernel: KernelConnectionMetadata }): Promise<void> {
1232+
const specOrModel = kernelConnectionMetadataHasKernelModel(data.kernel)
1233+
? data.kernel.kernelModel
1234+
: data.kernel.kernelSpec;
12291235
if (!this._notebook && specOrModel && this.notebookIdentity.resource.toString() === data.identity.toString()) {
1236+
const kernelSpecLanguage = kernelConnectionMetadataHasKernelSpec(data.kernel)
1237+
? data.kernel.kernelSpec?.language
1238+
: undefined;
12301239
// No notebook, send update to UI anyway
12311240
this.postMessage(InteractiveWindowMessages.UpdateKernel, {
12321241
jupyterServerStatus: ServerStatus.NotStarted,
12331242
localizedUri: '',
12341243
displayName: specOrModel?.display_name || specOrModel?.name || '',
1235-
language: translateKernelLanguageToMonaco(data.kernel.kernelSpec?.language ?? PYTHON_LANGUAGE)
1244+
language: translateKernelLanguageToMonaco(kernelSpecLanguage ?? PYTHON_LANGUAGE)
12361245
}).ignoreErrors();
12371246

12381247
// Update our model

src/client/datascience/interactive-common/notebookProvider.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { traceWarning } from '../../common/logger';
1111
import { IDisposableRegistry, Resource } from '../../common/types';
1212
import { noop } from '../../common/utils/misc';
1313
import { Identifiers } from '../constants';
14-
import { KernelSpecInterpreter } from '../jupyter/kernels/kernelSelector';
14+
import { KernelConnectionMetadata } from '../jupyter/kernels/types';
1515
import {
1616
ConnectNotebookProviderOptions,
1717
GetNotebookOptions,
@@ -28,7 +28,7 @@ export class NotebookProvider implements INotebookProvider {
2828
private _notebookCreated = new EventEmitter<{ identity: Uri; notebook: INotebook }>();
2929
private readonly _onSessionStatusChanged = new EventEmitter<{ status: ServerStatus; notebook: INotebook }>();
3030
private _connectionMade = new EventEmitter<void>();
31-
private _potentialKernelChanged = new EventEmitter<{ identity: Uri; kernel: KernelSpecInterpreter }>();
31+
private _potentialKernelChanged = new EventEmitter<{ identity: Uri; kernel: KernelConnectionMetadata }>();
3232
private _type: 'jupyter' | 'raw' = 'jupyter';
3333
public get activeNotebooks() {
3434
return [...this.notebooks.values()];
@@ -154,7 +154,7 @@ export class NotebookProvider implements INotebookProvider {
154154

155155
// This method is here so that the kernel selector can pick a kernel and not have
156156
// to know about any of the UI that's active.
157-
public firePotentialKernelChanged(identity: Uri, kernel: KernelSpecInterpreter) {
157+
public firePotentialKernelChanged(identity: Uri, kernel: KernelConnectionMetadata) {
158158
this._potentialKernelChanged.fire({ identity, kernel });
159159
}
160160

src/client/datascience/jupyter/jupyterExecution.ts

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ import {
3636
import { JupyterSelfCertsError } from './jupyterSelfCertsError';
3737
import { createRemoteConnectionInfo, expandWorkingDir } from './jupyterUtils';
3838
import { JupyterWaitForIdleError } from './jupyterWaitForIdleError';
39-
import { KernelSelector, KernelSpecInterpreter } from './kernels/kernelSelector';
39+
import { kernelConnectionMetadataHasKernelSpec } from './kernels/helpers';
40+
import { KernelSelector } from './kernels/kernelSelector';
41+
import { KernelConnectionMetadata } from './kernels/types';
4042
import { NotebookStarter } from './notebookStarter';
4143

4244
const LocalHosts = ['localhost', '127.0.0.1', '::1'];
@@ -141,8 +143,10 @@ export class JupyterExecutionBase implements IJupyterExecution {
141143
return Cancellation.race(async () => {
142144
let result: INotebookServer | undefined;
143145
let connection: IJupyterConnection | undefined;
144-
let kernelSpecInterpreter: KernelSpecInterpreter | undefined;
145-
let kernelSpecInterpreterPromise: Promise<KernelSpecInterpreter> = Promise.resolve({});
146+
let kernelConnectionMetadata: KernelConnectionMetadata | undefined;
147+
let kernelConnectionMetadataPromise: Promise<KernelConnectionMetadata | undefined> = Promise.resolve<
148+
KernelConnectionMetadata | undefined
149+
>(undefined);
146150
traceInfo(`Connecting to ${options ? options.purpose : 'unknown type of'} server`);
147151
const allowUI = !options || options.allowUI();
148152
const kernelSpecCancelSource = new CancellationTokenSource();
@@ -157,7 +161,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
157161
// Get hold of the kernelspec and corresponding (matching) interpreter that'll be used as the spec.
158162
// We can do this in parallel, while starting the server (faster).
159163
traceInfo(`Getting kernel specs for ${options ? options.purpose : 'unknown type of'} server`);
160-
kernelSpecInterpreterPromise = this.kernelSelector.getKernelForLocalConnection(
164+
kernelConnectionMetadataPromise = this.kernelSelector.getKernelForLocalConnection(
161165
undefined,
162166
'jupyter',
163167
undefined,
@@ -174,9 +178,9 @@ export class JupyterExecutionBase implements IJupyterExecution {
174178
while (tryCount <= maxTries && !this.disposed) {
175179
try {
176180
// Start or connect to the process
177-
[connection, kernelSpecInterpreter] = await Promise.all([
181+
[connection, kernelConnectionMetadata] = await Promise.all([
178182
this.startOrConnect(options, cancelToken),
179-
kernelSpecInterpreterPromise
183+
kernelConnectionMetadataPromise
180184
]);
181185

182186
if (!connection.localLaunch && LocalHosts.includes(connection.hostName.toLowerCase())) {
@@ -186,12 +190,17 @@ export class JupyterExecutionBase implements IJupyterExecution {
186190
result = this.serviceContainer.get<INotebookServer>(INotebookServer);
187191

188192
// In a remote non quest situation, figure out a kernel spec too.
189-
if (!kernelSpecInterpreter.kernelSpec && connection && !options?.skipSearchingForKernel) {
193+
if (
194+
(!kernelConnectionMetadata ||
195+
!kernelConnectionMetadataHasKernelSpec(kernelConnectionMetadata)) &&
196+
connection &&
197+
!options?.skipSearchingForKernel
198+
) {
190199
const sessionManagerFactory = this.serviceContainer.get<IJupyterSessionManagerFactory>(
191200
IJupyterSessionManagerFactory
192201
);
193202
const sessionManager = await sessionManagerFactory.create(connection);
194-
kernelSpecInterpreter = await this.kernelSelector.getKernelForRemoteConnection(
203+
kernelConnectionMetadata = await this.kernelSelector.getKernelForRemoteConnection(
195204
undefined,
196205
sessionManager,
197206
options?.metadata,
@@ -200,16 +209,14 @@ export class JupyterExecutionBase implements IJupyterExecution {
200209
await sessionManager.dispose();
201210
}
202211

203-
// If no kernel and not going to pick one, exit early
204-
if (!Object.keys(kernelSpecInterpreter) && !allowUI) {
205-
return undefined;
206-
}
207-
208212
// Populate the launch info that we are starting our server with
209213
const launchInfo: INotebookServerLaunchInfo = {
210214
connectionInfo: connection!,
211-
interpreter: kernelSpecInterpreter.interpreter,
212-
kernelSpec: kernelSpecInterpreter.kernelSpec,
215+
interpreter: kernelConnectionMetadata?.interpreter,
216+
kernelSpec:
217+
kernelConnectionMetadata && kernelConnectionMetadataHasKernelSpec(kernelConnectionMetadata)
218+
? kernelConnectionMetadata?.kernelSpec
219+
: kernelConnectionMetadata?.kernelModel,
213220
workingDir: options ? options.workingDir : undefined,
214221
uri: options ? options.uri : undefined,
215222
purpose: options ? options.purpose : uuid()
@@ -252,10 +259,9 @@ export class JupyterExecutionBase implements IJupyterExecution {
252259
cancelToken,
253260
launchInfo.kernelSpec?.display_name || launchInfo.kernelSpec?.name
254261
);
255-
if (Object.keys(kernelInterpreter).length > 0) {
262+
if (kernelInterpreter) {
256263
launchInfo.interpreter = kernelInterpreter.interpreter;
257-
launchInfo.kernelSpec =
258-
kernelInterpreter.kernelSpec || kernelInterpreter.kernelModel;
264+
launchInfo.kernelSpec = kernelInterpreter?.kernelSpec;
259265
continue;
260266
}
261267
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@ const NamedRegexp = require('named-js-regexp') as typeof import('named-js-regexp
1010

1111
// tslint:disable-next-line: no-require-imports
1212
import cloneDeep = require('lodash/cloneDeep');
13+
import {
14+
DefaultKernelConnectionMetadata,
15+
KernelConnectionMetadata,
16+
KernelSpecConnectionMetadata,
17+
LiveKernelConnectionMetadata,
18+
PythonKernelConnectionMetadata
19+
} from './types';
1320

1421
// Helper functions for dealing with kernels and kernelspecs
1522

@@ -23,6 +30,20 @@ export function findIndexOfConnectionFile(kernelSpec: Readonly<IJupyterKernelSpe
2330
return kernelSpec.argv.indexOf(connectionFilePlaceholder);
2431
}
2532

33+
type ConnectionWithKernelSpec =
34+
| KernelSpecConnectionMetadata
35+
| PythonKernelConnectionMetadata
36+
| DefaultKernelConnectionMetadata;
37+
export function kernelConnectionMetadataHasKernelSpec(
38+
connectionMetadata: KernelConnectionMetadata
39+
): connectionMetadata is ConnectionWithKernelSpec {
40+
return connectionMetadata.kind !== 'connectToLiveKernel';
41+
}
42+
export function kernelConnectionMetadataHasKernelModel(
43+
connectionMetadata: KernelConnectionMetadata
44+
): connectionMetadata is LiveKernelConnectionMetadata {
45+
return connectionMetadata.kind === 'connectToLiveKernel';
46+
}
2647
// Create a default kernelspec with the given display name
2748
export function createDefaultKernelSpec(displayName?: string): IJupyterKernelSpec {
2849
// This creates a default kernel spec. When launched, 'python' argument will map to using the interpreter

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,15 @@ import {
3636
InterruptResult,
3737
KernelSocketInformation
3838
} from '../../types';
39+
import { kernelConnectionMetadataHasKernelModel } from './helpers';
3940
import { KernelExecution } from './kernelExecution';
40-
import type { IKernel, IKernelProvider, IKernelSelectionUsage, KernelSelection, LiveKernelModel } from './types';
41+
import type {
42+
IKernel,
43+
IKernelProvider,
44+
IKernelSelectionUsage,
45+
KernelConnectionMetadata,
46+
LiveKernelModel
47+
} from './types';
4148

4249
export class Kernel implements IKernel {
4350
get connection(): INotebookProviderConnection | undefined {
@@ -47,7 +54,9 @@ export class Kernel implements IKernel {
4754
if (this.notebook) {
4855
return this.notebook.getKernelSpec();
4956
}
50-
return this.metadata.kernelSpec || this.metadata.kernelModel;
57+
return kernelConnectionMetadataHasKernelModel(this.metadata)
58+
? this.metadata.kernelModel
59+
: this.metadata.kernelSpec;
5160
}
5261
get onStatusChanged(): Event<ServerStatus> {
5362
return this._onStatusChanged.event;
@@ -81,7 +90,7 @@ export class Kernel implements IKernel {
8190
private startCancellation = new CancellationTokenSource();
8291
constructor(
8392
public readonly uri: Uri,
84-
public readonly metadata: Readonly<KernelSelection>,
93+
public readonly metadata: Readonly<KernelConnectionMetadata>,
8594
private readonly notebookProvider: INotebookProvider,
8695
private readonly disposables: IDisposableRegistry,
8796
private readonly launchTimeout: number,
@@ -143,10 +152,12 @@ export class Kernel implements IKernel {
143152
} else {
144153
await this.validate(this.uri);
145154
const metadata = ((getDefaultNotebookContent().metadata || {}) as unknown) as nbformat.INotebookMetadata;
155+
// tslint:disable-next-line: no-suspicious-comment
156+
// TODO: Just pass the `this.metadata` into the func.
146157
updateNotebookMetadata(
147158
metadata,
148159
this.metadata.interpreter,
149-
this.metadata.kernelSpec || this.metadata.kernelModel
160+
this.metadata.kind === 'connectToLiveKernel' ? this.metadata.kernelModel : this.metadata.kernelSpec
150161
);
151162

152163
this._notebookPromise = this.notebookProvider.getOrCreateNotebook({

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export class KernelExecution implements IDisposable {
156156
interpreter: activeInterpreter!,
157157
kernelModel: undefined,
158158
kernelSpec: undefined,
159-
kind: 'pythonInterpreter'
159+
kind: 'startUsingPythonInterpreter'
160160
},
161161
launchingFile: document.uri.fsPath
162162
});

0 commit comments

Comments
 (0)