From 078896a1666096dbaa54e800e781938e4658b762 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Sat, 21 Sep 2019 03:32:24 -0700 Subject: [PATCH] Update kernel status to have a connection status and a kernel status. This simplified logic quite a bit. Most of these ideas came from previous work in #4724 --- packages/console/src/widget.ts | 25 +- packages/help-extension/src/index.tsx | 6 +- packages/notebook/src/panel.ts | 4 +- packages/services/src/kernel/default.ts | 400 ++++++++++++---------- packages/services/src/kernel/kernel.ts | 101 +++--- packages/services/src/kernel/manager.ts | 2 +- packages/services/src/serverconnection.ts | 2 + 7 files changed, 290 insertions(+), 250 deletions(-) diff --git a/packages/console/src/widget.ts b/packages/console/src/widget.ts index 199845174a65..f91bb7a0091c 100644 --- a/packages/console/src/widget.ts +++ b/packages/console/src/widget.ts @@ -774,38 +774,23 @@ export class CodeConsole extends Widget { /** * Handle a change to the kernel. */ - private _onKernelChanged(): void { + private async _onKernelChanged(): Promise { this.clear(); if (this._banner) { this._banner.dispose(); this._banner = null; } this.addBanner(); + this._handleInfo(await this.session.kernel.info); } /** * Handle a change to the kernel status. */ - private _onKernelStatusChanged(): void { - if (this.session.status === 'connected') { - // we just had a kernel restart or reconnect - reset banner - let kernel = this.session.kernel; - if (!kernel) { - return; - } - kernel.ready - .then(() => { - if (this.isDisposed || !kernel || !kernel.info) { - return; - } - this._handleInfo(this.session.kernel.info); - }) - .catch(err => { - console.error('could not get kernel info'); - }); - } else if (this.session.status === 'restarting') { + private async _onKernelStatusChanged(): Promise { + if (this.session.status === 'restarting') { this.addBanner(); - this._handleInfo(this.session.kernel.info); + this._handleInfo(await this.session.kernel.info); } } diff --git a/packages/help-extension/src/index.tsx b/packages/help-extension/src/index.tsx index 9c3f1d0dcd8d..734c59f98fa7 100644 --- a/packages/help-extension/src/index.tsx +++ b/packages/help-extension/src/index.tsx @@ -190,7 +190,8 @@ function activate( return; } const session = serviceManager.sessions.connectTo(sessionModel); - void session.kernel.ready.then(() => { + + void session.kernel.info.then(kernelInfo => { // Check the cache second time so that, if two callbacks get scheduled, // they don't try to add the same commands. if (kernelInfoCache.has(sessionModel.kernel.name)) { @@ -198,7 +199,6 @@ function activate( } // Set the Kernel Info cache. const name = session.kernel.name; - const kernelInfo = session.kernel.info; kernelInfoCache.set(name, kernelInfo); // Utility function to check if the current widget @@ -265,7 +265,7 @@ function activate( // Add the kernel info help_links to the Help menu. const kernelGroup: Menu.IItemOptions[] = []; - (session.kernel.info.help_links || []).forEach(link => { + (kernelInfo.help_links || []).forEach(link => { const commandId = `help-menu-${name}:${link.text}`; commands.addCommand(commandId, { label: link.text, diff --git a/packages/notebook/src/panel.ts b/packages/notebook/src/panel.ts index 92cbad7cac87..acad0013415c 100644 --- a/packages/notebook/src/panel.ts +++ b/packages/notebook/src/panel.ts @@ -196,9 +196,9 @@ export class NotebookPanel extends DocumentWidget { return; } let { newValue } = args; - void newValue.ready.then(() => { + void newValue.info.then(info => { if (this.model && this.context.session.kernel === newValue) { - this._updateLanguage(newValue.info.language_info); + this._updateLanguage(info.language_info); } }); void this._updateSpec(newValue); diff --git a/packages/services/src/kernel/default.ts b/packages/services/src/kernel/default.ts index 266b4828e8c7..48c35731b3dd 100644 --- a/packages/services/src/kernel/default.ts +++ b/packages/services/src/kernel/default.ts @@ -64,19 +64,32 @@ export class DefaultKernel implements Kernel.IKernel { this.handleComms = options.handleComms === undefined ? true : options.handleComms; - void this._readyPromise.promise.then(() => { - this._sendPending(); + this.connectionStatusChanged.connect((sender, connectionStatus) => { + // Send pending messages and update status to be consistent. + if (connectionStatus === 'connected' && this.status !== 'dead') { + // Make sure we send at least one message to get kernel status back. + if (this._pendingMessages.length > 0) { + this._sendPending(); + } else { + void this.requestKernelInfo(); + } + } else { + // If the connection is down, then we don't know what is happening with + // the kernel, so set the status to unknown. + this._updateStatus('unknown'); + } }); this._createSocket(); + + // Immediately queue up a request for initial kernel info. + void this.requestKernelInfo(); + Private.runningKernels.push(this); } - /** - * A signal emitted when the kernel is shut down. - */ - get terminated(): ISignal { - return this._terminated; + get disposed(): ISignal { + return this._disposed; } /** @@ -103,6 +116,13 @@ export class DefaultKernel implements Kernel.IKernel { return this._statusChanged; } + /** + * A signal emitted when the kernel status changes. + */ + get connectionStatusChanged(): ISignal { + return this._connectionStatusChanged; + } + /** * A signal emitted for iopub kernel messages. * @@ -131,8 +151,15 @@ export class DefaultKernel implements Kernel.IKernel { * This signal is emitted when a message is received, before it is handled * asynchronously. * - * The behavior is undefined if the message is modified during message - * handling. As such, the message should be treated as read-only. + * This message is emitted when a message is queued for sending (either in + * the websocket buffer, or our own pending message buffer). The message may + * actually be sent across the wire at a later time. + * + * The message emitted in this signal should not be modified in any way. + * + * TODO: should we only emit copies of the message? Is that prohibitively + * expensive? Note that we can't just do a JSON copy since the message may + * include binary buffers. We could minimally copy the top-level, though. */ get anyMessage(): ISignal { return this._anyMessage; @@ -181,34 +208,24 @@ export class DefaultKernel implements Kernel.IKernel { } /** - * Test whether the kernel has been disposed. + * The current connection status of the kernel connection. */ - get isDisposed(): boolean { - return this._isDisposed; + get connectionStatus(): Kernel.ConnectionStatus { + return this._connectionStatus; } /** - * The cached kernel info. - * - * #### Notes - * This value will be null until the kernel is ready. - */ - get info(): KernelMessage.IInfoReply | null { - return this._info; - } - - /** - * Test whether the kernel is ready. + * Test whether the kernel has been disposed. */ - get isReady(): boolean { - return this._isReady; + get isDisposed(): boolean { + return this._isDisposed; } /** - * A promise that is fulfilled when the kernel is ready. + * The cached kernel info. */ - get ready(): Promise { - return this._readyPromise.promise; + get info(): Promise { + return this._info.promise; } /** @@ -235,6 +252,8 @@ export class DefaultKernel implements Kernel.IKernel { name: this._name, username: this._username, serverSettings: this.serverSettings, + // TODO: should this be !this.handleComms, because we only want one + // connection to handle comms? handleComms: this.handleComms, ...options }, @@ -250,10 +269,11 @@ export class DefaultKernel implements Kernel.IKernel { return; } this._isDisposed = true; - this._terminated.emit(); - this._status = 'dead'; - // Trigger the async _clearState, but do not wait for it. - void this._clearState(); + this._disposed.emit(); + + // Took out this._status = 'dead' - figure out ramifications of this. + this._updateConnectionStatus('disconnected'); + this._clearState(); this._clearSocket(); this._kernelSession = ''; this._msgChain = null; @@ -337,15 +357,9 @@ export class DefaultKernel implements Kernel.IKernel { KernelMessage.IShellControlMessage, KernelMessage.IShellControlMessage > { - if (this.status === 'dead') { - throw new Error('Kernel is dead'); - } - if (!this._isReady || !this._ws) { - this._pendingMessages.push(msg); - } else { - this._ws.send(serialize.serialize(msg)); - } + this._sendMessage(msg); this._anyMessage.emit({ msg, direction: 'send' }); + let future = new ctor( () => { let msgId = msg.header.msg_id; @@ -381,6 +395,33 @@ export class DefaultKernel implements Kernel.IKernel { return future; } + /** + * Send a message on the websocket. + * + * If pending is true, queue the message for later sending if we cannot send + * now. Otherwise throw an error. + */ + private _sendMessage(msg: KernelMessage.IMessage, pending = true) { + if (this.status === 'dead') { + throw new Error('Kernel is dead'); + } + + if (this.connectionStatus === 'disconnected') { + throw new Error('Kernel connection is disconnected'); + } + + // Send if the ws allows it, otherwise buffer the message. + if (this.connectionStatus === 'connected') { + this._ws.send(serialize.serialize(msg)); + console.log(`SENT WS message to ${this.id}`, msg); + } else if (pending) { + this._pendingMessages.push(msg); + console.log(`PENDING WS message to ${this.id}`, msg); + } else { + throw new Error('Could not send message'); + } + } + /** * Interrupt a kernel. * @@ -399,14 +440,18 @@ export class DefaultKernel implements Kernel.IKernel { } /** - * Restart a kernel. + * Request a kernel restart. * * #### Notes - * Uses the [Jupyter Notebook API](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/jupyter/notebook/master/notebook/services/api/api.yaml#!/kernels) and validates the response model. + * Uses the [Jupyter Notebook API](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/jupyter/notebook/master/notebook/services/api/api.yaml#!/kernels) + * and validates the response model. * - * Any existing Future or Comm objects are cleared. + * Any existing Future or Comm objects are cleared once the kernel has + * actually be restarted. * - * The promise is fulfilled on a valid response and rejected otherwise. + * The promise is fulfilled on a valid server response and rejected otherwise. + * Note that this does not mean the kernel has been restarted, only that a + * restart has been requested. * * It is assumed that the API call does not mutate the kernel id or name. * @@ -431,12 +476,30 @@ export class DefaultKernel implements Kernel.IKernel { * * #### Notes * Used when the websocket connection to the kernel is lost. + * + * TODO: should we remove this method? It doesn't play well with + * 'disconnected' being a terminal state. We already have the automatic + * reconnection. */ reconnect(): Promise { + this._updateConnectionStatus('connecting'); + let result = new PromiseDelegate(); + + // TODO: can we use one of the test functions that has this pattern of a + // promise resolving on a single signal emission? + let fulfill = (sender: this, status: Kernel.ConnectionStatus) => { + if (status === 'connected') { + result.resolve(); + } else if (status === 'disconnected') { + result.reject(new Error('Kernel connection disconnected')); + } + this.connectionStatusChanged.disconnect(fulfill); + }; + this.connectionStatusChanged.connect(fulfill); + this._clearSocket(); - this._updateStatus('reconnecting'); this._createSocket(); - return this._readyPromise.promise; + return result.promise; } /** @@ -451,16 +514,19 @@ export class DefaultKernel implements Kernel.IKernel { * object, and fulfills the promise. * * If the kernel is already `dead`, it closes the websocket and returns - * without a server request. + * without a server request.a */ async shutdown(): Promise { + // TODO: review the logic here. Why are the clear statements in different orders? if (this.status === 'dead') { + this._updateConnectionStatus('disconnected'); this._clearSocket(); await this._clearState(); return; } await Private.shutdownKernel(this.id, this.serverSettings); await this._clearState(); + this._updateConnectionStatus('disconnected'); this._clearSocket(); } @@ -488,13 +554,22 @@ export class DefaultKernel implements Kernel.IKernel { if (this.isDisposed) { throw new Error('Disposed kernel'); } - // Relax the requirement that a kernel info reply has a status attribute, - // since this part of the spec is not yet widely conformed-to. - if (reply.content.status && reply.content.status !== 'ok') { + // Kernels sometimes do not include a status field on kernel_info_reply + // messages, so set a default for now. + // See https://github.com/jupyterlab/jupyterlab/issues/6760 + if (reply.content.status === undefined) { + reply.content.status = 'ok'; + } + + if (reply.content.status !== 'ok') { throw new Error('Kernel info reply errored'); } - // Type assertion is necessary until we can rely on the status message. - this._info = reply.content as KernelMessage.IInfoReply; + + // TODO: Since we never make another _info promise delegate, this only + // resolves to the *first* return, and ignores all calls after that. Is + // that what we want, or do we want this info updated when another request + // is made? + this._info.resolve(reply.content); return reply; } @@ -705,9 +780,6 @@ export class DefaultKernel implements Kernel.IKernel { * See [Messaging in Jupyter](https://jupyter-client.readthedocs.io/en/latest/messaging.html#messages-on-the-stdin-router-dealer-sockets). */ sendInputReply(content: KernelMessage.IInputReplyMsg['content']): void { - if (this.status === 'dead') { - throw new Error('Kernel is dead'); - } let msg = KernelMessage.createMessage({ msgType: 'input_reply', channel: 'stdin', @@ -715,11 +787,8 @@ export class DefaultKernel implements Kernel.IKernel { session: this._clientId, content }); - if (!this._isReady || !this._ws) { - this._pendingMessages.push(msg); - } else { - this._ws.send(serialize.serialize(msg)); - } + + this._sendMessage(msg); this._anyMessage.emit({ msg, direction: 'send' }); } @@ -923,12 +992,14 @@ export class DefaultKernel implements Kernel.IKernel { /** * Clear the socket state. * + * TODO: does this not apply anymore. + * * #### Notes - * When calling this, you should also set the status to something like - * 'reconnecting' to reset the kernel ready state. + * When calling this, you should also set the connectionStatus to + * 'connecting' if you are going to try to reconnect, or 'disconnected' if + * not. */ private _clearSocket(): void { - this._wsStopped = true; if (this._ws !== null) { // Clear the websocket event handlers and the socket itself. this._ws.onopen = this._noOp; @@ -944,57 +1015,17 @@ export class DefaultKernel implements Kernel.IKernel { * Handle status iopub messages from the kernel. */ private _updateStatus(status: Kernel.Status): void { - switch (status) { - case 'idle': - case 'busy': - if (!this._isReady && this._initialized) { - this._isReady = true; - this._readyPromise.resolve(); - } - break; - case 'restarting': - // Send a kernel_info_request to get to a known kernel state. - void this.requestKernelInfo().catch(this._noOp); - break; - case 'starting': - case 'autorestarting': - // 'starting' can happen at initialization or 'restarting'. - // 'autorestarting' is always preceded by 'restarting'. In either case, - // the 'restarting' handler above is fine, so we do nothing here. - /* no-op */ - break; - case 'connected': - // requestKernelInfo is sent by the onWSOpen - break; - case 'reconnecting': - if (this._isReady) { - this._isReady = false; - this._readyPromise = new PromiseDelegate(); - void this._readyPromise.promise.then(() => { - // when we are ready again, send any pending messages. - this._sendPending(); - }); - } - break; - case 'dead': - if (this._isReady) { - this._isReady = false; - this._readyPromise = new PromiseDelegate(); - } - void this._readyPromise.promise.catch(this._noOp); - this._readyPromise.reject('Kernel is dead'); - break; - default: - console.error('invalid kernel status:', status); - return; + // "unknown" | "starting" | "idle" | "busy" | "restarting" | "autorestarting" | "dead" + + if (this._status === status || this._status === 'dead') { + return; } - if (status !== this._status) { - this._status = status; - Private.logKernelStatus(this); - this._statusChanged.emit(status); - if (status === 'dead') { - this.dispose(); - } + + this._status = status; + Private.logKernelStatus(this); + this._statusChanged.emit(status); + if (status === 'dead') { + this.dispose(); } } @@ -1002,12 +1033,22 @@ export class DefaultKernel implements Kernel.IKernel { * Send pending messages to the kernel. */ private _sendPending(): void { - // We shift the message off the queue - // after the message is sent so that if there is an exception, - // the message is still pending. - while (this._ws && this._pendingMessages.length > 0) { - let msg = serialize.serialize(this._pendingMessages[0]); - this._ws.send(msg); + // We check to make sure we are still connected each time. For + // example, if a websocket buffer overflows, it may close, so we should + // stop sending messages. + while ( + this.connectionStatus === 'connected' && + this._pendingMessages.length > 0 + ) { + this._sendMessage(this._pendingMessages[0], false); + // TODO: remove debuging console.log below. + // console.log( + // `SENT pending message to ${this.id}`, + // this._pendingMessages[0] + // ); + + // We shift the message off the queue after the message is sent so that + // if there is an exception, the message is still pending. this._pendingMessages.shift(); } } @@ -1160,7 +1201,7 @@ export class DefaultKernel implements Kernel.IKernel { // Strip any authentication from the display string. // TODO - Audit tests for extra websockets started let display = partialUrl.replace(/^((?:\w+:)?\/\/)(?:[^@\/]+@)/, '$1'); - console.log('Starting WebSocket:', display); + console.log(`Starting WebSocket: ${display}`); let url = URLExt.join( partialUrl, @@ -1172,7 +1213,6 @@ export class DefaultKernel implements Kernel.IKernel { url = url + `&token=${encodeURIComponent(token)}`; } - this._wsStopped = false; this._ws = new settings.WebSocket(url); // Ensure incoming binary messages are not Blobs @@ -1189,38 +1229,13 @@ export class DefaultKernel implements Kernel.IKernel { */ private _onWSOpen = (evt: Event) => { this._reconnectAttempt = 0; - this._updateStatus('connected'); - - // We temporarily set the ready status to true so our kernel info request - // below will go through. - this._isReady = true; - - // Get the kernel info, signaling that the kernel is ready. - this.requestKernelInfo() - .then(() => { - this._initialized = true; - this._isReady = true; - this._readyPromise.resolve(); - }) - .catch(err => { - this._initialized = true; - this._readyPromise.reject(err); - }); - - // Reset the isReady status after we sent our message so others wait for - // the kernel info request to come back. - this._isReady = false; + this._updateConnectionStatus('connected'); }; /** * Handle a websocket message, validating and routing appropriately. */ private _onWSMessage = (evt: MessageEvent) => { - if (this._wsStopped) { - // If the socket is being closed, ignore any messages - return; - } - // Notify immediately if there is an error with the message. let msg: KernelMessage.IMessage; try { @@ -1252,6 +1267,28 @@ export class DefaultKernel implements Kernel.IKernel { this._anyMessage.emit({ msg, direction: 'recv' }); }; + /** + * Handle connection status changes. + * + * #### Notes + * The 'disconnected' state is considered a terminal state. + */ + private _updateConnectionStatus( + connectionStatus: Kernel.ConnectionStatus + ): void { + if ( + this._connectionStatus === connectionStatus || + this._connectionStatus === 'disconnected' + ) { + return; + } + + this._connectionStatus = connectionStatus; + + // Notify others that the connection status changed. + this._connectionStatusChanged.emit(connectionStatus); + } + private async _handleMessage(msg: KernelMessage.IMessage): Promise { let handled = false; @@ -1339,14 +1376,9 @@ export class DefaultKernel implements Kernel.IKernel { * Handle a websocket close event. */ private _onWSClose = (evt: Event) => { - if (this._wsStopped || !this._ws) { - return; - } - // Clear the websocket event handlers and the socket itself. - this._clearSocket(); - + // Update the connection status and schedule a possible reconnection. if (this._reconnectAttempt < this._reconnectLimit) { - this._updateStatus('reconnecting'); + this._updateConnectionStatus('connecting'); let timeout = Math.pow(2, this._reconnectAttempt); console.error( 'Connection lost, reconnecting in ' + timeout + ' seconds.' @@ -1354,24 +1386,28 @@ export class DefaultKernel implements Kernel.IKernel { setTimeout(this._createSocket, 1e3 * timeout); this._reconnectAttempt += 1; } else { - this._updateStatus('dead'); + this._updateConnectionStatus('disconnected'); } + + // Clear the websocket event handlers and the socket itself. + this._clearSocket(); }; private _id = ''; private _name = ''; private _status: Kernel.Status = 'unknown'; + private _connectionStatus: Kernel.ConnectionStatus = 'connecting'; private _kernelSession = ''; private _clientId = ''; private _isDisposed = false; - private _wsStopped = false; + /** + * Websocket to communicate with kernel. + */ private _ws: WebSocket | null = null; private _username = ''; private _reconnectLimit = 7; private _reconnectAttempt = 0; - private _isReady = false; - private _readyPromise = new PromiseDelegate(); - private _initialized = false; + private _futures = new Map< string, KernelFutureHandler< @@ -1386,16 +1422,19 @@ export class DefaultKernel implements Kernel.IKernel { msg: KernelMessage.ICommOpenMsg ) => void; } = Object.create(null); - private _info: KernelMessage.IInfoReply | null = null; + private _info = new PromiseDelegate(); private _pendingMessages: KernelMessage.IMessage[] = []; private _specPromise: Promise; private _statusChanged = new Signal(this); + private _connectionStatusChanged = new Signal( + this + ); + private _disposed = new Signal(this); private _iopubMessage = new Signal(this); private _anyMessage = new Signal(this); private _unhandledMessage = new Signal(this); private _displayIdToParentIds = new Map(); private _msgIdToDisplayIds = new Map(); - private _terminated = new Signal(this); private _msgChain: Promise | null = Promise.resolve(); private _noOp = () => { /* no-op */ @@ -1413,20 +1452,19 @@ export namespace DefaultKernel { * * @param settings - The optional server settings. * - * @returns A promise that resolves with the model for the kernel. + * @returns A promise that resolves with the model for the kernel, or undefined if not found. * * #### Notes * If the kernel was already started via `startNewKernel`, we return its * `Kernel.IModel`. * * Otherwise, we attempt to find an existing kernel by connecting to the - * server. The promise is fulfilled when the kernel is found, otherwise the - * promise is rejected. + * server. */ export function findById( id: string, settings?: ServerConnection.ISettings - ): Promise { + ): Promise { return Private.findById(id, settings); } @@ -1554,20 +1592,29 @@ namespace Private { * Find a kernel by id. * * Will reach out to the server if needed to find the kernel. + * + * Returns undefined if the kernel is not found. */ - export function findById( + export async function findById( id: string, settings?: ServerConnection.ISettings - ): Promise { - let kernel = find(runningKernels, value => { - return value.id === id; - }); + ): Promise { + let kernel = find(runningKernels, value => value.id === id); if (kernel) { - return Promise.resolve(kernel.model); + return kernel.model; + } + try { + return getKernelModel(id, settings); + } catch (e) { + // If the error is a 404, the session wasn't found. + if ( + e instanceof ServerConnection.ResponseError && + e.response.status === 404 + ) { + return undefined; + } + throw e; } - return getKernelModel(id, settings).catch(() => { - throw new Error(`No running kernel with id: ${id}`); - }); } /** @@ -1647,9 +1694,8 @@ namespace Private { kernels: Kernel.IModel[] ): Kernel.IModel[] { each(runningKernels.slice(), kernel => { - let updated = find(kernels, model => { - return kernel.id === model.id; - }); + let updated = find(kernels, model => kernel.id === model.id); + // If kernel is no longer running on disk, emit dead signal. if (!updated && kernel.status !== 'dead') { kernel.dispose(); diff --git a/packages/services/src/kernel/kernel.ts b/packages/services/src/kernel/kernel.ts index 86d1f523cbf4..4e92fc80ebf8 100644 --- a/packages/services/src/kernel/kernel.ts +++ b/packages/services/src/kernel/kernel.ts @@ -5,7 +5,7 @@ import { IIterator } from '@phosphor/algorithm'; import { JSONObject, JSONValue } from '@phosphor/coreutils'; -import { IDisposable } from '@phosphor/disposable'; +import { IDisposable, IObservableDisposable } from '@phosphor/disposable'; import { ISignal } from '@phosphor/signaling'; @@ -38,7 +38,7 @@ export namespace Kernel { * kernel is changed, the object itself can take care of disconnecting and * reconnecting listeners. */ - export interface IKernelConnection extends IDisposable { + export interface IKernelConnection extends IObservableDisposable { /** * The id of the server-side kernel. */ @@ -78,31 +78,9 @@ export namespace Kernel { readonly connectionStatus: Kernel.ConnectionStatus; /** - * The cached kernel info. - * - * #### Notes - * This value will be null until the kernel is ready. - */ - readonly info: KernelMessage.IInfoReply | null; - - /** - * Test whether the kernel is ready. - * - * #### Notes - * A kernel is ready when the communication channel is active and we have - * cached the kernel info. + * The kernel info */ - readonly isReady: boolean; - - /** - * A promise that resolves when the kernel is initially ready after a start - * or restart. - * - * #### Notes - * A kernel is ready when the communication channel is active and we have - * cached the kernel info. - */ - readonly ready: Promise; + readonly info: Promise; /** * Whether the kernel connection handles comm messages. @@ -163,6 +141,7 @@ export namespace Kernel { expectReply?: boolean, disposeOnDone?: boolean ): Kernel.IControlFuture>; + /** * Reconnect to a disconnected kernel. * @@ -462,21 +441,16 @@ export namespace Kernel { msgId: string, hook: (msg: KernelMessage.IIOPubMessage) => boolean | PromiseLike ): void; - } - /** - * The full interface of a kernel. - */ - export interface IKernel extends IKernelConnection { /** - * A signal emitted when the kernel is shut down. + * A signal emitted when the kernel status changes. */ - terminated: ISignal; + statusChanged: ISignal; /** - * A signal emitted when the kernel status changes. + * A signal emitted when the kernel connection status changes. */ - statusChanged: ISignal; + connectionStatusChanged: ISignal; /** * A signal emitted after an iopub kernel message is handled. @@ -497,7 +471,12 @@ export namespace Kernel { * message should be treated as read-only. */ anyMessage: ISignal; + } + /** + * The full interface of a kernel. + */ + export interface IKernel extends IKernelConnection { /** * The server settings for the kernel. */ @@ -512,8 +491,8 @@ export namespace Kernel { * Uses the [Jupyter Notebook * API](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/jupyter/notebook/master/notebook/services/api/api.yaml#!/kernels). * - * On a valid response, closes the websocket, emits the [[terminated]] - * signal, disposes of the kernel object, and fulfills the promise. + * On a valid response, closes the websocket, disposes of the kernel + * object, and fulfills the promise. * * The promise will be rejected if the kernel status is `'dead'`, the * request fails, or the response is invalid. @@ -532,14 +511,12 @@ export namespace Kernel { * * #### Notes * If the kernel was already started via `startNewKernel`, we return its - * `Kernel.IModel`. Otherwise, we attempt to find the existing kernel. The - * promise is fulfilled when the kernel is found, otherwise the promise is - * rejected. + * `Kernel.IModel`. Otherwise, we attempt to find the existing kernel. */ export function findById( id: string, settings?: ServerConnection.ISettings - ): Promise { + ): Promise { return DefaultKernel.findById(id, settings); } @@ -701,6 +678,7 @@ export namespace Kernel { /** * A signal emitted when there is a connection failure. + * TODO: figure out the relationship between this and the other connection status signals for kernels. */ connectionFailure: ISignal; @@ -772,10 +750,10 @@ export namespace Kernel { * Find a kernel by id. * * @param id - The id of the target kernel. - * - * @returns A promise that resolves with the kernel's model. + * TODO: should we return undefined or reject if we can't find the model? + * @returns A promise that resolves with the kernel's model, or undefined if not found. */ - findById(id: string): Promise; + findById(id: string): Promise; /** * Connect to an existing kernel. @@ -1009,17 +987,46 @@ export namespace Kernel { /** * The valid Kernel status states. + * + * #### Notes + * The status states are: + * * `unknown`: The kernel status is unkown, often because the connection is + * disconnected or connecting. This state is determined by the kernel + * connection status. + * * `starting`: The kernel is starting + * * `idle`: The kernel has finished processing messages. + * * `busy`: The kernel is currently processing messages. + * * `restarting`: The kernel is restarting. This state is sent by the Jupyter + * server. + * * `dead`: The kernel is dead and will not be restarted. This state is set + * by the Jupyter server and is a final state. */ export type Status = | 'unknown' | 'starting' - | 'reconnecting' | 'idle' | 'busy' | 'restarting' | 'autorestarting' - | 'dead' - | 'connected'; + | 'dead'; + + /** + * The valid kernel connection states. + * + * #### Notes + * The status states are: + * * `connected`: The kernel connection is live. + * * `connecting`: The kernel connection is not live, but we are attempting + * to reconnect to the kernel. + * * `disconnected`: The kernel connection is permanently down, we will not + * try to reconnect. + * + * When a kernel connection is `connected`, the kernel status should be + * valid. When a kernel connection is either `connecting` or `disconnected`, + * the kernel status will be `unknown` unless the kernel status was `dead`, + * in which case it stays `dead`. + */ + export type ConnectionStatus = 'connected' | 'connecting' | 'disconnected'; /** * The kernel model provided by the server. diff --git a/packages/services/src/kernel/manager.ts b/packages/services/src/kernel/manager.ts index ec2c407a3b66..96e730f4876c 100644 --- a/packages/services/src/kernel/manager.ts +++ b/packages/services/src/kernel/manager.ts @@ -333,7 +333,7 @@ export class KernelManager implements Kernel.IManager { this._models.push(kernel.model); this._runningChanged.emit(this._models.slice()); } - kernel.terminated.connect(() => { + kernel.disposed.connect(() => { this._onTerminated(id); }); } diff --git a/packages/services/src/serverconnection.ts b/packages/services/src/serverconnection.ts index 8d1b203976e7..36e9257249ba 100644 --- a/packages/services/src/serverconnection.ts +++ b/packages/services/src/serverconnection.ts @@ -279,6 +279,8 @@ namespace Private { // Convert the TypeError into a more specific error. throw new ServerConnection.NetworkError(e); }); + // TODO: *this* is probably where we need a system-wide connectionFailure + // signal we can hook into. } /**