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

Rework services terminal code #7674

Merged
merged 18 commits into from Dec 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions examples/terminal/src/index.ts
Expand Up @@ -12,7 +12,7 @@ import '../index.css';

import { DockPanel, Widget } from '@lumino/widgets';

import { TerminalSession } from '@jupyterlab/services';
import { TerminalManager } from '@jupyterlab/services';

import { Terminal } from '@jupyterlab/terminal';

Expand All @@ -28,12 +28,13 @@ async function main(): Promise<void> {
dock.fit();
});

const s1 = await TerminalSession.startNew();
const manager = new TerminalManager();
const s1 = await manager.startNew();
const term1 = new Terminal(s1, { theme: 'light' });
term1.title.closable = true;
dock.addWidget(term1);

const s2 = await TerminalSession.startNew();
const s2 = await manager.startNew();
const term2 = new Terminal(s2, { theme: 'dark' });
term2.title.closable = true;
dock.addWidget(term2, { mode: 'tab-before' });
Expand Down
4 changes: 3 additions & 1 deletion packages/docregistry/src/context.ts
Expand Up @@ -548,7 +548,9 @@ export class Context<T extends DocumentRegistry.IModel>
throw err;
}
)
.catch();
.catch(() => {
/* no-op */
});
}

/**
Expand Down
7 changes: 4 additions & 3 deletions packages/services/examples/browser/src/terminal.ts
@@ -1,17 +1,18 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

import { TerminalSession } from '@jupyterlab/services';
import { Terminal, TerminalManager } from '@jupyterlab/services';

import { log } from './log';

export async function main() {
log('Terminal');

// See if terminals are available
if (TerminalSession.isAvailable()) {
if (Terminal.isAvailable()) {
let manager = new TerminalManager();
// Create a named terminal session and send some data.
let session = await TerminalSession.startNew();
let session = await manager.startNew();
session.send({ type: 'stdin', content: ['foo'] });
}
}
54 changes: 28 additions & 26 deletions packages/services/src/kernel/default.ts
Expand Up @@ -52,22 +52,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
this._username = options.username ?? '';
this.handleComms = options.handleComms ?? true;

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.
Expand Down Expand Up @@ -264,7 +248,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
this._updateConnectionStatus('disconnected');
this._clearKernelState();
this._clearSocket();
clearTimeout(this._reconnectTimeout);

// Clear Lumino signals
Signal.clearData(this);
Expand Down Expand Up @@ -398,10 +381,8 @@ export class KernelConnection implements Kernel.IKernelConnection {
// 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 (queue) {
this._pendingMessages.push(msg);
// console.log(`PENDING WS message to ${this.id}`, msg);
} else {
throw new Error('Could not send message');
}
Expand Down Expand Up @@ -1034,8 +1015,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
* Handle status iopub messages from the kernel.
*/
private _updateStatus(status: KernelMessage.Status): void {
// "unknown" | "starting" | "idle" | "busy" | "restarting" | "autorestarting" | "dead"

if (this._status === status || this._status === 'dead') {
return;
}
Expand Down Expand Up @@ -1194,14 +1173,16 @@ export class KernelConnection implements Kernel.IKernelConnection {

/**
* Create the kernel websocket connection and add socket status handlers.
*
* #### Notes
* You are responsible for clearing the old socket so that the handlers
* from it are gone. For example, calling ._clearSocket(), etc.
*/
private _createSocket = () => {
this._errorIfDisposed();

// Make sure the socket is clear
this._clearSocket();

// Update the connection status to reflect opening a new connection.
this._updateConnectionStatus('connecting');

let settings = this.serverSettings;
let partialUrl = URLExt.join(
settings.wsUrl,
Expand Down Expand Up @@ -1246,6 +1227,28 @@ export class KernelConnection implements Kernel.IKernelConnection {

this._connectionStatus = connectionStatus;

// If we are not 'connecting', reset any reconnection attempts.
if (connectionStatus !== 'connecting') {
this._reconnectAttempt = 0;
clearTimeout(this._reconnectTimeout);
}

if (this.status !== 'dead') {
if (connectionStatus === 'connected') {
// Send pending messages, and 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 do not know what is happening
// with the kernel, so set the status to unknown.
this._updateStatus('unknown');
}
}

// Notify others that the connection status changed.
this._connectionStatusChanged.emit(connectionStatus);
}
Expand Down Expand Up @@ -1386,7 +1389,6 @@ export class KernelConnection implements Kernel.IKernelConnection {
* Handle a websocket open event.
*/
private _onWSOpen = (evt: Event) => {
this._reconnectAttempt = 0;
this._updateConnectionStatus('connected');
};

Expand Down
11 changes: 11 additions & 0 deletions packages/services/src/kernel/manager.ts
Expand Up @@ -87,6 +87,9 @@ export class KernelManager extends BaseManager implements Kernel.IManager {
* Dispose of the resources used by the manager.
*/
dispose(): void {
if (this.isDisposed) {
return;
}
this._models.clear();
this._kernelConnections.forEach(x => x.dispose());
this._pollModels.dispose();
Expand Down Expand Up @@ -292,6 +295,14 @@ export class KernelManager extends BaseManager implements Kernel.IManager {

private _onDisposed(kernelConnection: KernelConnection) {
this._kernelConnections.delete(kernelConnection);
// A dispose emission could mean the server session is deleted, or that
// the kernel JS object is disposed and the kernel still exists on the
// server, so we refresh from the server to make sure we reflect the
// server state.

void this.refreshRunning().catch(() => {
/* no-op */
});
}

private _onStatusChanged(
Expand Down
4 changes: 2 additions & 2 deletions packages/services/src/manager.ts
Expand Up @@ -19,7 +19,7 @@ import { Session, SessionManager } from './session';

import { Setting, SettingManager } from './setting';

import { TerminalSession, TerminalManager } from './terminal';
import { Terminal, TerminalManager } from './terminal';

import { ServerConnection } from './serverconnection';

Expand Down Expand Up @@ -219,7 +219,7 @@ export namespace ServiceManager {
/**
* The terminals manager for the manager.
*/
readonly terminals: TerminalSession.IManager;
readonly terminals: Terminal.IManager;

/**
* The workspace manager for the manager.
Expand Down
3 changes: 3 additions & 0 deletions packages/services/src/session/manager.ts
Expand Up @@ -87,6 +87,9 @@ export class SessionManager extends BaseManager implements Session.IManager {
* Dispose of the resources used by the manager.
*/
dispose(): void {
if (this.isDisposed) {
return;
}
this._models.clear();
this._sessionConnections.forEach(x => x.dispose());
this._pollModels.dispose();
Expand Down