Skip to content

Commit dd82fc3

Browse files
rchiodorchiodo
authored
Support ipywidgets in the kernel work (microsoft#11444)
* Refactor to have socket information on the session * Basic idea building * Working but super slow * Working without ipywidgets * Remapped to not use enchannelConnection * Implement dispose support * Fixup serialization * Working with widgets * Fix output to be more usable * Add a bunch of tests and fixup shutdown problems * Fix logging test * pruneCell fixes and enabling for ipywidget tests * Fix requests for CDN * Fix jupyter session to have socket information * Fix prune problem * More logging and case insensitive compare for file path * Fix interrupt * Remove unused rewire * Fix sonar problems * Try again with exclusions * Add comment about post install * Review feedback * Make script path check work on linux too * Fix tests * Fix new log message * Try a different way to get the build to pass * Better way to remove output * More warnings * More warnings * Session unit tsts are more trouble than they're worth. Socket is hanging unit tests. Just eliminate them as their functionality is already covered by functional tests.
1 parent b2a0b59 commit dd82fc3

37 files changed

Lines changed: 2209 additions & 2939 deletions

‎.sonarcloud.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
sonar.sources=src/client,src/datascience-ui
22
sonar.tests=src/test
33
sonar.cfamily.build-wrapper-output.bypass=true
4-
sonar.cpd.exclusions=src/datascience-ui/**/redux/actions.ts
4+
sonar.cpd.exclusions=src/datascience-ui/**/redux/actions.ts,src/client/**/raw-kernel/rawKernel.ts

‎build/ci/postInstall.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,32 @@ function fixJupyterLabDTSFiles() {
4747
console.log(colors.red(relativePath + ' file does not need updating.'));
4848
}
4949
}
50+
51+
/**
52+
* In order to get raw kernels working, we reuse the default kernel that jupyterlab ships.
53+
* However it expects to be talking to a websocket which is serializing the messages to strings.
54+
* Our raw kernel is not a web socket and needs to do its own serialization. To do so, we make a copy
55+
* of the default kernel with the serialization stripped out. This is simpler than making a copy of the module
56+
* at runtime.
57+
*/
58+
function createJupyterKernelWithoutSerialization() {
59+
var relativePath = path.join('node_modules', '@jupyterlab', 'services', 'lib', 'kernel', 'default.js');
60+
var filePath = path.join(constants_1.ExtensionRootDir, relativePath);
61+
if (!fs.existsSync(filePath)) {
62+
throw new Error("Jupyter lab default kernel not found '" + filePath + "' (pvsc post install script)");
63+
}
64+
var fileContents = fs.readFileSync(filePath, { encoding: 'utf8' });
65+
var replacedContents = fileContents.replace(
66+
/^const serialize =.*$/gm,
67+
'const serialize = { serialize: (a) => a, deserialize: (a) => a };'
68+
);
69+
if (replacedContents === fileContents) {
70+
throw new Error('Jupyter lab default kernel cannot be made non serializing');
71+
}
72+
var destPath = path.join(path.dirname(filePath), 'nonSerializingKernel.js');
73+
fs.writeFileSync(destPath, replacedContents);
74+
console.log(colors.green(destPath + ' file generated (by Python VSC)'));
75+
}
76+
5077
fixJupyterLabDTSFiles();
78+
createJupyterKernelWithoutSerialization();

‎build/webpack/webpack.extension.config.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ const config = {
8989
// ZMQ requires prebuilds to be in our node_modules directory. So recreate the ZMQ structure.
9090
// However we don't webpack to manage this, so it was part of the excluded modules. Delete it from there
9191
// so at runtime we pick up the original structure.
92-
new removeFilesWebpackPlugin({ after: { include: ['./out/client/node_modules/zeromq.js'] } }),
92+
new removeFilesWebpackPlugin({ after: { include: ['./out/client/node_modules/zeromq.js'], log: false } }),
9393
new copyWebpackPlugin([{ from: './node_modules/zeromq/**/*.js' }]),
9494
new copyWebpackPlugin([{ from: './node_modules/zeromq/**/*.node' }]),
9595
new copyWebpackPlugin([{ from: './node_modules/zeromq/**/*.json' }]),

‎gulpfile.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,11 @@ function getAllowedWarningsForWebPack(buildConfig) {
296296
'WARNING in ./node_modules/ws/lib/buffer-util.js',
297297
'WARNING in ./node_modules/ws/lib/Validation.js',
298298
'WARNING in ./node_modules/ws/lib/validation.js',
299-
'WARNING in ./node_modules/any-promise/register.js'
299+
'WARNING in ./node_modules/any-promise/register.js',
300+
'remove-files-plugin@1.4.0:',
301+
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/buffer-util.js',
302+
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/validation.js',
303+
'WARNING in ./node_modules/@jupyterlab/services/node_modules/ws/lib/Validation.js'
300304
];
301305
case 'debugAdapter':
302306
return ['WARNING in ./node_modules/vscode-uri/lib/index.js'];

‎package.nls.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
"DataScience.startingJupyter": "Starting Jupyter server",
137137
"DataScience.connectingToJupyter": "Connecting to Jupyter server",
138138
"DataScience.connectingToIPyKernel": "Connecting to IPython kernel",
139+
"DataScience.connectedToIPyKernel": "Connected.",
139140
"Experiments.inGroup": "User belongs to experiment group '{0}'",
140141
"Interpreters.RefreshingInterpreters": "Refreshing Python Interpreters",
141142
"Interpreters.entireWorkspace": "Entire workspace",
@@ -486,5 +487,6 @@
486487
"DataScience.widgetScriptNotFoundOnCDNWidgetMightNotWork": "Unable to load a compatible version of the widget '{0}'. Expected behavior may be affected.",
487488
"DataScience.unhandledMessage": "Unhandled kernel message from a widget: {0} : {1}",
488489
"DataScience.qgridWidgetScriptVersionCompatibilityWarning": "Unable to load a compatible version of the widget 'qgrid'. Consider downgrading to version 1.1.1.",
490+
"DataScience.kernelStarted": "Started kernel {0}",
489491
"DataScience.ipykernelNotInstalled": "Ipykernel is not installed."
490492
}

‎src/client/common/utils/localize.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,7 @@ export namespace DataScience {
400400
export const importingFormat = localize('DataScience.importingFormat', 'Importing {0}');
401401
export const startingJupyter = localize('DataScience.startingJupyter', 'Starting Jupyter server');
402402
export const connectingIPyKernel = localize('DataScience.connectingToIPyKernel', 'Connecting to IPython kernel');
403+
export const connectedToIPyKernel = localize('DataScience.connectedToIPyKernel', 'Connected.');
403404
export const connectingToJupyter = localize('DataScience.connectingToJupyter', 'Connecting to Jupyter server');
404405
export const exportingFormat = localize('DataScience.exportingFormat', 'Exporting {0}');
405406
export const runAllCellsLensCommandTitle = localize(
@@ -917,6 +918,8 @@ export namespace DataScience {
917918
'DataScience.qgridWidgetScriptVersionCompatibilityWarning',
918919
"Unable to load a compatible version of the widget 'qgrid'. Consider downgrading to version 1.1.1."
919920
);
921+
922+
export const kernelStarted = localize('DataScience.kernelStarted', 'Started kernel {0}.');
920923
export const ipykernelNotInstalled = localize('DataScience.ipykernelNotInstalled', 'Ipykernel is not installed.');
921924
}
922925

‎src/client/datascience/baseJupyterSession.ts‎

Lines changed: 42 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -9,27 +9,17 @@ import { ReplaySubject } from 'rxjs/ReplaySubject';
99
import { Event, EventEmitter } from 'vscode';
1010
import { CancellationToken } from 'vscode-jsonrpc';
1111
import { ServerStatus } from '../../datascience-ui/interactive-common/mainState';
12-
import { isTestExecution } from '../common/constants';
1312
import { traceError, traceInfo, traceWarning } from '../common/logger';
14-
import { IDisposable } from '../common/types';
1513
import { waitForPromise } from '../common/utils/async';
1614
import * as localize from '../common/utils/localize';
1715
import { noop } from '../common/utils/misc';
1816
import { sendTelemetryEvent } from '../telemetry';
1917
import { Telemetry } from './constants';
20-
import { JupyterWebSockets } from './jupyter/jupyterWebSocket';
2118
import { JupyterKernelPromiseFailedError } from './jupyter/kernels/jupyterKernelPromiseFailedError';
2219
import { KernelSelector } from './jupyter/kernels/kernelSelector';
2320
import { LiveKernelModel } from './jupyter/kernels/types';
24-
import { IKernelProcess } from './kernel-launcher/types';
25-
import { IJupyterKernelSpec, IJupyterSession, KernelSocketInformation } from './types';
26-
27-
export type ISession = Session.ISession & {
28-
// Whether this is a remote session that we attached to.
29-
isRemoteSession?: boolean;
30-
// If a kernel process is associated with this session
31-
process?: IKernelProcess;
32-
};
21+
import { suppressShutdownErrors } from './raw-kernel/rawKernel';
22+
import { IJupyterKernelSpec, IJupyterSession, ISessionWithSocket, KernelSocketInformation } from './types';
3323

3424
/**
3525
* Exception raised when starting a Jupyter Session fails.
@@ -47,49 +37,9 @@ export class JupyterSessionStartError extends Error {
4737
}
4838

4939
export abstract class BaseJupyterSession implements IJupyterSession {
50-
protected get session(): ISession | undefined {
40+
protected get session(): ISessionWithSocket | undefined {
5141
return this._session;
5242
}
53-
protected set session(session: ISession | undefined) {
54-
const oldSession = this._session;
55-
this._session = session;
56-
57-
// When setting the session clear our current exit handler and hook up to the
58-
// new session process
59-
if (this.processExitHandler) {
60-
this.processExitHandler.dispose();
61-
this.processExitHandler = undefined;
62-
}
63-
if (session?.process) {
64-
// Watch to see if our process exits
65-
this.processExitHandler = session.process.exited((exitCode) => {
66-
traceError(`Raw kernel process exited code: ${exitCode}`);
67-
this.shutdown().catch((reason) => {
68-
traceError(`Error shutting down jupyter session: ${reason}`);
69-
});
70-
// Next code the user executes will show a session disposed message
71-
});
72-
}
73-
74-
// If we have a new session, then emit the new kernel connection information.
75-
if (session && oldSession !== session) {
76-
const socket = JupyterWebSockets.get(session.kernel.id);
77-
if (!socket) {
78-
traceError(`Unable to find WebSocket connetion assocated with kernel ${session.kernel.id}`);
79-
this._kernelSocket.next(undefined);
80-
return;
81-
}
82-
this._kernelSocket.next({
83-
options: {
84-
clientId: session.kernel.clientId,
85-
id: session.kernel.id,
86-
model: { ...session.kernel.model },
87-
userName: session.kernel.username
88-
},
89-
socket: socket
90-
});
91-
}
92-
}
9343
protected kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined;
9444
public get kernelSocket(): Observable<KernelSocketInformation | undefined> {
9545
return this._kernelSocket;
@@ -117,33 +67,23 @@ export abstract class BaseJupyterSession implements IJupyterSession {
11767
return this.connected;
11868
}
11969
protected onStatusChangedEvent: EventEmitter<ServerStatus> = new EventEmitter<ServerStatus>();
120-
protected statusHandler: Slot<ISession, Kernel.Status>;
70+
protected statusHandler: Slot<ISessionWithSocket, Kernel.Status>;
12171
protected connected: boolean = false;
122-
protected restartSessionPromise: Promise<ISession | undefined> | undefined;
123-
private _session: ISession | undefined;
72+
protected restartSessionPromise: Promise<ISessionWithSocket | undefined> | undefined;
73+
private _session: ISessionWithSocket | undefined;
12474
private _kernelSocket = new ReplaySubject<KernelSocketInformation | undefined>();
12575
private _jupyterLab?: typeof import('@jupyterlab/services');
126-
private processExitHandler: IDisposable | undefined;
12776

12877
constructor(protected readonly kernelSelector: KernelSelector) {
12978
this.statusHandler = this.onStatusChanged.bind(this);
13079
}
13180
public dispose(): Promise<void> {
132-
if (this.processExitHandler) {
133-
this.processExitHandler.dispose();
134-
this.processExitHandler = undefined;
135-
}
13681
return this.shutdown();
13782
}
13883
// Abstracts for each Session type to implement
13984
public abstract async waitForIdle(timeout: number): Promise<void>;
14085

14186
public async shutdown(): Promise<void> {
142-
if (this.processExitHandler) {
143-
this.processExitHandler.dispose();
144-
this.processExitHandler = undefined;
145-
}
146-
14787
if (this.session) {
14888
try {
14989
traceInfo('Shutdown session - current session');
@@ -157,7 +97,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
15797
} catch {
15898
noop();
15999
}
160-
this.session = undefined;
100+
this.setSession(undefined);
161101
this.restartSessionPromise = undefined;
162102
}
163103
if (this.onStatusChangedEvent) {
@@ -179,7 +119,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
179119
}
180120

181121
public async changeKernel(kernel: IJupyterKernelSpec | LiveKernelModel, timeoutMS: number): Promise<void> {
182-
let newSession: ISession | undefined;
122+
let newSession: ISessionWithSocket | undefined;
183123

184124
// If we are already using this kernel in an active session just return back
185125
if (this.kernelSpec?.name === kernel.name && this.session) {
@@ -198,13 +138,13 @@ export abstract class BaseJupyterSession implements IJupyterSession {
198138
this.kernelSpec = kernel;
199139

200140
// Save the new session
201-
this.session = newSession;
141+
this.setSession(newSession);
202142

203143
// Listen for session status changes
204144
this.session?.statusChanged.connect(this.statusHandler); // NOSONAR
205145

206146
// Start the restart session promise too.
207-
this.restartSessionPromise = this.createRestartSession(kernel, this.session);
147+
this.restartSessionPromise = this.createRestartSession(kernel, newSession);
208148
}
209149

210150
public async restart(_timeout: number): Promise<void> {
@@ -227,7 +167,7 @@ export abstract class BaseJupyterSession implements IJupyterSession {
227167
const oldStatusHandler = this.statusHandler;
228168

229169
// Just switch to the other session. It should already be ready
230-
this.session = await this.restartSessionPromise;
170+
this.setSession(await this.restartSessionPromise);
231171
if (!this.session) {
232172
throw new Error(localize.DataScience.sessionDisposed());
233173
}
@@ -357,19 +297,42 @@ export abstract class BaseJupyterSession implements IJupyterSession {
357297
protected abstract startRestartSession(): void;
358298
protected abstract async createRestartSession(
359299
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined,
360-
session: ISession,
300+
session: ISessionWithSocket,
361301
cancelToken?: CancellationToken
362-
): Promise<ISession>;
302+
): Promise<ISessionWithSocket>;
363303

364304
// Sub classes need to implement their own kernel change specific code
365305
protected abstract createNewKernelSession(
366306
kernel: IJupyterKernelSpec | LiveKernelModel,
367307
timeoutMS: number
368-
): Promise<ISession>;
308+
): Promise<ISessionWithSocket>;
309+
310+
// Changes the current session.
311+
protected setSession(session: ISessionWithSocket | undefined) {
312+
const oldSession = this._session;
313+
this._session = session;
369314

315+
// If we have a new session, then emit the new kernel connection information.
316+
if (session && oldSession !== session) {
317+
if (!session.kernelSocketInformation) {
318+
traceError(`Unable to find WebSocket connection assocated with kernel ${session.kernel.id}`);
319+
this._kernelSocket.next(undefined);
320+
} else {
321+
this._kernelSocket.next({
322+
options: {
323+
clientId: session.kernel.clientId,
324+
id: session.kernel.id,
325+
model: { ...session.kernel.model },
326+
userName: session.kernel.username
327+
},
328+
socket: session.kernelSocketInformation.socket
329+
});
330+
}
331+
}
332+
}
370333
protected async shutdownSession(
371-
session: ISession | undefined,
372-
statusHandler: Slot<ISession, Kernel.Status> | undefined
334+
session: ISessionWithSocket | undefined,
335+
statusHandler: Slot<ISessionWithSocket, Kernel.Status> | undefined
373336
): Promise<void> {
374337
if (session && session.kernel) {
375338
const kernelId = session.kernel.id;
@@ -384,30 +347,9 @@ export abstract class BaseJupyterSession implements IJupyterSession {
384347
return;
385348
}
386349
try {
387-
// When running under a test, mark all futures as done so we
388-
// don't hit this problem:
389-
// https://github.com/jupyterlab/jupyterlab/issues/4252
390-
// tslint:disable:no-any
391-
if (isTestExecution()) {
392-
const defaultKernel = session.kernel as any;
393-
if (defaultKernel && defaultKernel._futures) {
394-
const futures = defaultKernel._futures as Map<any, any>;
395-
if (futures) {
396-
futures.forEach((f) => {
397-
if (f._status !== undefined) {
398-
f._status |= 4;
399-
}
400-
});
401-
}
402-
}
403-
if (defaultKernel && defaultKernel._reconnectLimit) {
404-
defaultKernel._reconnectLimit = 0;
405-
}
406-
await waitForPromise(session.shutdown(), 1000);
407-
} else {
408-
// Shutdown may fail if the process has been killed
409-
await waitForPromise(session.shutdown(), 1000);
410-
}
350+
suppressShutdownErrors(session.kernel);
351+
// Shutdown may fail if the process has been killed
352+
await waitForPromise(session.shutdown(), 1000);
411353
} catch {
412354
noop();
413355
}

‎src/client/datascience/common.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export function pruneCell(cell: nbformat.ICell): nbformat.ICell {
9393
delete result.execution_count;
9494
} else {
9595
// Clean outputs from code cells
96-
result.outputs = (result.outputs as nbformat.IOutput[]).map(fixupOutput);
96+
result.outputs = result.outputs ? (result.outputs as nbformat.IOutput[]).map(fixupOutput) : [];
9797
}
9898

9999
return result;

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { inject, injectable } from 'inversify';
77
import { ConfigurationChangeEvent, Event, EventEmitter, TextEditor, Uri, WindowState } from 'vscode';
88
import { IApplicationShell, IDocumentManager, IWorkspaceService } from '../../common/application/types';
99
import '../../common/extensions';
10-
import { traceError } from '../../common/logger';
1110
import { IFileSystem } from '../../common/platform/types';
1211
import { IDisposable } from '../../common/types';
1312
import { INotebookIdentity, InteractiveWindowMessages } from '../interactive-common/interactiveWindowTypes';
@@ -66,11 +65,6 @@ export class AutoSaveService implements IInteractiveWindowListener {
6665
} else if (message === InteractiveWindowMessages.LoadAllCellsComplete) {
6766
const notebook = this.getNotebook();
6867
if (!notebook) {
69-
traceError(
70-
`Received message ${message}, but there is no notebook for ${
71-
this.notebookUri ? this.notebookUri.fsPath : undefined
72-
}`
73-
);
7468
return;
7569
}
7670
this.disposables.push(notebook.modified(this.onNotebookModified, this, this.disposables));

‎src/client/datascience/ipywidgets/ipyWidgetMessageDispatcher.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ export class IPyWidgetMessageDispatcher implements IIPyWidgetMessageDispatcher {
259259
}
260260
while (this.pendingMessages.length) {
261261
try {
262-
this.kernelSocketInfo.socket?.send(this.pendingMessages[0]); // NOSONAR
262+
this.kernelSocketInfo.socket?.sendToRealKernel(this.pendingMessages[0]); // NOSONAR
263263
this.pendingMessages.shift();
264264
} catch (ex) {
265265
traceError('Failed to send message to Kernel', ex);

0 commit comments

Comments
 (0)