Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notify the user when a notebook kernel autorestarts #6246

Merged
merged 35 commits into from May 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
51c3ad9
Make a new ‘autorestarting’ kernel status for restarts we didn’t cause.
jasongrout Apr 25, 2019
6ebe08b
In a kernel restart for a console, use the existing kernel info for t…
jasongrout Apr 25, 2019
78abc44
Do not kill the websocket connection on kernel restart.
jasongrout May 1, 2019
290c4db
Fix kernel status indicator to only show busy when it is actually busy.
jasongrout May 1, 2019
3233965
Make sure the console shows a new banner when restarting.
jasongrout May 1, 2019
579c7af
Make sure the notebook language update is not obsolete by the time we…
jasongrout May 1, 2019
463af7a
Pop up a dialog on autorestarts.
jasongrout May 2, 2019
8cac49d
More work in progress to simplify state management on restart.
jasongrout May 4, 2019
365b77e
When kernel is done initializing, make sure to set the isReady state …
jasongrout May 7, 2019
b236b45
test describe functions should never be async (and should not return …
jasongrout May 7, 2019
b9f2f15
WIP: Skip tests that aren’t working
jasongrout May 13, 2019
7d31d6a
Fix console error.
jasongrout May 13, 2019
0f3b24f
For CI testing, do not bail on first error.
jasongrout May 13, 2019
d4b4a82
Rewrite dialog test utilities with optional arguments and async/await.
jasongrout May 13, 2019
1d40ecd
rewrite restartKernel function to be async/await
jasongrout May 14, 2019
2f63f65
Be more careful about future done promises being wrapped up when clea…
jasongrout May 14, 2019
4b9a418
When restarting, handle a requestKernelInfo that rejects.
jasongrout May 14, 2019
39599db
Toolbar tests are passing
jasongrout May 14, 2019
5cd23e6
More tests that are actually fine.
jasongrout May 14, 2019
76b1ebc
Send pending kernel messages via the kernel ready promise.
jasongrout May 14, 2019
08474e9
Handle the ‘connected’ kernel status differently.
jasongrout May 14, 2019
6d1865a
Schedule sending pending kernel messages for the initial ready promis…
jasongrout May 14, 2019
549e7c9
Clean up kernel code (no functionality changes).
jasongrout May 14, 2019
a50a25e
Skipped future test now passes!
jasongrout May 14, 2019
130d44d
Skipped console test now passes!
jasongrout May 14, 2019
e770060
Skipped comm tests now pass!
jasongrout May 14, 2019
8885ecd
Catch the kernel status dead rejection so we don’t have an unhandled …
jasongrout May 15, 2019
a044cae
ikernel tests pass now.
jasongrout May 15, 2019
f5f7f5c
Clean up tests, stop skipping a few that work again.
jasongrout May 15, 2019
bab8017
Throw error when shutting down a dead kernel, like its docs claim.
jasongrout May 15, 2019
3e20b14
Handle promises with a void.
jasongrout May 15, 2019
bcb9210
Integrity fix.
jasongrout May 15, 2019
2884cf5
Fix broken clientsession tests.
jasongrout May 15, 2019
0743bb2
Delete TODO notes about restarts timing out.
jasongrout May 15, 2019
8ea8e92
Revert shutdown behavior when kernel is dead, and change documentatio…
jasongrout May 15, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -26,7 +26,7 @@
"clean:src": "jlpm run clean",
"clean:test": "lerna run clean --scope \"@jupyterlab/test-*\"",
"clean:utils": "cd buildutils && jlpm run clean",
"coverage": "lerna run coverage --scope \"@jupyterlab/test-*\" --stream --concurrency 1",
"coverage": "lerna run coverage --scope \"@jupyterlab/test-*\" --stream --concurrency 1 --no-bail",
"create:package": "node buildutils/lib/create-package.js",
"create:test": "node buildutils/lib/create-test-package.js",
"create:theme": "node buildutils/lib/create-theme.js",
Expand Down
23 changes: 11 additions & 12 deletions packages/apputils/src/clientsession.tsx
Expand Up @@ -878,26 +878,25 @@ export namespace ClientSession {
*
* Returns a promise resolving with whether the kernel was restarted.
*/
export function restartKernel(
export async function restartKernel(
kernel: Kernel.IKernelConnection
): Promise<boolean> {
let restartBtn = Dialog.warnButton({ label: 'RESTART ' });
return showDialog({
const result = await showDialog({
title: 'Restart Kernel?',
body:
'Do you want to restart the current kernel? All variables will be lost.',
buttons: [Dialog.cancelButton(), restartBtn]
}).then(result => {
if (kernel.isDisposed) {
return Promise.resolve(false);
}
if (result.button.accept) {
return kernel.restart().then(() => {
return true;
});
}
return false;
});

if (kernel.isDisposed) {
return false;
}
if (result.button.accept) {
await kernel.restart();
return true;
}
return false;
}

/**
Expand Down
19 changes: 15 additions & 4 deletions packages/apputils/src/toolbar.tsx
Expand Up @@ -3,6 +3,8 @@

import { UseSignal, ReactWidget } from './vdom';

import { Kernel } from '@jupyterlab/services';

import { Button } from '@jupyterlab/ui-components';

import { IIterator, find, map, some } from '@phosphor/algorithm';
Expand Down Expand Up @@ -400,8 +402,7 @@ export namespace Toolbar {
* Create a kernel status indicator item.
*
* #### Notes
* It show display a busy status if the kernel status is
* not idle.
* It will show a busy status if the kernel status is busy.
* It will show the current status in the node title.
* It can handle a change to the context or the kernel.
*/
Expand Down Expand Up @@ -679,10 +680,20 @@ namespace Private {
return;
}
let status = session.status;
this.toggleClass(TOOLBAR_IDLE_CLASS, status === 'idle');
this.toggleClass(TOOLBAR_BUSY_CLASS, status !== 'idle');
const busy = this._isBusy(status);
this.toggleClass(TOOLBAR_BUSY_CLASS, busy);
this.toggleClass(TOOLBAR_IDLE_CLASS, !busy);
let title = 'Kernel ' + status[0].toUpperCase() + status.slice(1);
this.node.title = title;
}

/**
* Check if status should be shown as busy.
*/
private _isBusy(status: Kernel.Status): boolean {
return (
status === 'busy' || status === 'starting' || status === 'restarting'
);
}
}
}
4 changes: 2 additions & 2 deletions packages/console/src/widget.ts
Expand Up @@ -784,8 +784,7 @@ export class CodeConsole extends Widget {
if (!kernel) {
return;
}
kernel
.requestKernelInfo()
kernel.ready
.then(() => {
if (this.isDisposed || !kernel || !kernel.info) {
return;
Expand All @@ -797,6 +796,7 @@ export class CodeConsole extends Widget {
});
} else if (this.session.status === 'restarting') {
this.addBanner();
this._handleInfo(this.session.kernel.info);
}
}

Expand Down
46 changes: 44 additions & 2 deletions packages/notebook/src/panel.ts
Expand Up @@ -9,7 +9,12 @@ import { Message } from '@phosphor/messaging';

import { ISignal, Signal } from '@phosphor/signaling';

import { IClientSession, Printing } from '@jupyterlab/apputils';
import {
IClientSession,
Printing,
showDialog,
Dialog
} from '@jupyterlab/apputils';

import { DocumentWidget } from '@jupyterlab/docregistry';

Expand Down Expand Up @@ -51,6 +56,10 @@ export class NotebookPanel extends DocumentWidget<Notebook, INotebookModel> {
// Set up things related to the context
this.content.model = this.context.model;
this.context.session.kernelChanged.connect(this._onKernelChanged, this);
this.context.session.statusChanged.connect(
this._onSessionStatusChanged,
this
);

void this.revealed.then(() => {
// Set the document edit mode on initial open if it looks like a new document.
Expand Down Expand Up @@ -184,13 +193,40 @@ export class NotebookPanel extends DocumentWidget<Notebook, INotebookModel> {
}
let { newValue } = args;
void newValue.ready.then(() => {
if (this.model) {
if (this.model && this.context.session.kernel === newValue) {
this._updateLanguage(newValue.info.language_info);
}
});
void this._updateSpec(newValue);
}

private _onSessionStatusChanged(
sender: IClientSession,
status: Kernel.Status
) {
// If the status is autorestarting, and we aren't already in a series of
// autorestarts, show the dialog.
if (status === 'autorestarting' && !this._autorestarting) {
// The kernel died and the server is restarting it. We notify the user so
// they know why their kernel state is gone.
void showDialog({
title: 'Kernel Restarting',
body: `The kernel for ${
this.session.path
} appears to have died. It will restart automatically.`,
buttons: [Dialog.okButton()]
});
this._autorestarting = true;
} else if (status === 'restarting') {
// Another autorestart attempt will first change the status to
// restarting, then to autorestarting again, so we don't reset the
// autorestarting status if the status is 'restarting'.
/* no-op */
} else {
this._autorestarting = false;
}
}

/**
* Update the kernel language.
*/
Expand All @@ -215,6 +251,12 @@ export class NotebookPanel extends DocumentWidget<Notebook, INotebookModel> {
}

private _activated = new Signal<this, void>(this);

/**
* Whether we are currently in a series of autorestarts we have already
* notified the user about.
*/
private _autorestarting = false;
}

/**
Expand Down