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

Make session.kernelChanged signal include both the old and new kernels. #4834

Merged
merged 1 commit into from
Jul 4, 2018
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions packages/apputils/src/clientsession.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface IClientSession extends IDisposable {
/**
* A signal emitted when the kernel changes.
*/
readonly kernelChanged: ISignal<this, Kernel.IKernelConnection | null>;
readonly kernelChanged: ISignal<this, Session.IKernelChangedArgs>;

/**
* A signal emitted when the kernel status changes.
Expand Down Expand Up @@ -230,7 +230,7 @@ export class ClientSession implements IClientSession {
/**
* A signal emitted when the kernel changes.
*/
get kernelChanged(): ISignal<this, Kernel.IKernelConnection | null> {
get kernelChanged(): ISignal<this, Session.IKernelChangedArgs> {
return this._kernelChanged;
}

Expand Down Expand Up @@ -672,8 +672,11 @@ export class ClientSession implements IClientSession {
session.statusChanged.connect(this._onStatusChanged, this);
session.iopubMessage.connect(this._onIopubMessage, this);
session.unhandledMessage.connect(this._onUnhandledMessage, this);
this._kernelChanged.emit(session.kernel);
this._prevKernelName = session.kernel.name;

// The session kernel was disposed above when the session was disposed, so
// the oldValue should be null.
this._kernelChanged.emit({ oldValue: null, newValue: session.kernel });
return session.kernel;
}

Expand Down Expand Up @@ -743,8 +746,11 @@ export class ClientSession implements IClientSession {
/**
* Handle a change to the kernel.
*/
private _onKernelChanged(sender: Session.ISession): void {
this._kernelChanged.emit(sender.kernel);
private _onKernelChanged(
sender: Session.ISession,
args: Session.IKernelChangedArgs
): void {
this._kernelChanged.emit(args);
}

/**
Expand Down Expand Up @@ -801,9 +807,7 @@ export class ClientSession implements IClientSession {
private _initializing = false;
private _isReady = false;
private _terminated = new Signal<this, void>(this);
private _kernelChanged = new Signal<this, Kernel.IKernelConnection | null>(
this
);
private _kernelChanged = new Signal<this, Session.IKernelChangedArgs>(this);
private _statusChanged = new Signal<this, Kernel.Status>(this);
private _iopubMessage = new Signal<this, KernelMessage.IMessage>(this);
private _unhandledMessage = new Signal<this, KernelMessage.IMessage>(this);
Expand Down
13 changes: 7 additions & 6 deletions packages/notebook/src/panel.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.

import { Kernel, KernelMessage } from '@jupyterlab/services';
import { Kernel, KernelMessage, Session } from '@jupyterlab/services';

import { Token } from '@phosphor/coreutils';

Expand Down Expand Up @@ -131,17 +131,18 @@ export class NotebookPanel extends DocumentWidget<Notebook, INotebookModel> {
*/
private _onKernelChanged(
sender: any,
kernel: Kernel.IKernelConnection
args: Session.IKernelChangedArgs
): void {
if (!this.model || !kernel) {
if (!this.model || !args.newValue) {
return;
}
kernel.ready.then(() => {
let { newValue } = args;
newValue.ready.then(() => {
if (this.model) {
this._updateLanguage(kernel.info.language_info);
this._updateLanguage(newValue.info.language_info);
}
});
this._updateSpec(kernel);
this._updateSpec(newValue);
}

/**
Expand Down
11 changes: 6 additions & 5 deletions packages/services/src/session/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export class DefaultSession implements Session.ISession {
/**
* A signal emitted when the kernel changes.
*/
get kernelChanged(): ISignal<this, Kernel.IKernelConnection> {
get kernelChanged(): ISignal<this, Session.IKernelChangedArgs> {
return this._kernelChanged;
}

Expand Down Expand Up @@ -196,9 +196,10 @@ export class DefaultSession implements Session.ISession {
this._type = model.type;

if (this._kernel.isDisposed || model.kernel.id !== this._kernel.id) {
let kernel = Kernel.connectTo(model.kernel, this.serverSettings);
this.setupKernel(kernel);
this._kernelChanged.emit(kernel);
let newValue = Kernel.connectTo(model.kernel, this.serverSettings);
let oldValue = this._kernel;
this.setupKernel(newValue);
this._kernelChanged.emit({ oldValue, newValue });
this._handleModelChange(oldModel);
return;
}
Expand Down Expand Up @@ -404,7 +405,7 @@ export class DefaultSession implements Session.ISession {
private _kernel: Kernel.IKernel;
private _isDisposed = false;
private _updating = false;
private _kernelChanged = new Signal<this, Kernel.IKernelConnection>(this);
private _kernelChanged = new Signal<this, Session.IKernelChangedArgs>(this);
private _statusChanged = new Signal<this, Kernel.Status>(this);
private _iopubMessage = new Signal<this, KernelMessage.IIOPubMessage>(this);
private _unhandledMessage = new Signal<this, KernelMessage.IMessage>(this);
Expand Down
16 changes: 15 additions & 1 deletion packages/services/src/session/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export namespace Session {
/**
* A signal emitted when the kernel changes.
*/
kernelChanged: ISignal<this, Kernel.IKernelConnection>;
kernelChanged: ISignal<this, IKernelChangedArgs>;

/**
* A signal emitted when the session status changes.
Expand Down Expand Up @@ -348,6 +348,20 @@ export namespace Session {
clientId?: string;
}

/**
* An arguments object for the kernel changed signal.
*/
export interface IKernelChangedArgs {
/**
* The old kernel.
*/
oldValue: Kernel.IKernelConnection | null;
/**
* The new kernel.
*/
newValue: Kernel.IKernelConnection | null;
}

/**
* Object which manages session instances.
*
Expand Down
12 changes: 8 additions & 4 deletions packages/services/test/src/session/session.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,16 +227,20 @@ describe('session', () => {

context('#kernelChanged', () => {
it('should emit when the kernel changes', async () => {
let called = false;
let called: Session.IKernelChangedArgs | null = null;
let object = {};
defaultSession.kernelChanged.connect((s, kernel) => {
called = true;
defaultSession.kernelChanged.connect((s, args) => {
called = args;
Signal.disconnectReceiver(object);
}, object);
let previous = defaultSession.kernel;
await defaultSession.changeKernel({ name: previous.name });
await defaultSession.kernel.ready;
expect(called).to.be(true);
expect(previous).to.not.be(defaultSession.kernel);
expect(called).to.eql({
oldValue: previous,
newValue: defaultSession.kernel
});
previous.dispose();
});
});
Expand Down
2 changes: 1 addition & 1 deletion tests/test-apputils/src/clientsession.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('@jupyterlab/apputils', () => {
it('should be emitted when the kernel changes', done => {
session.kernelChanged.connect((sender, args) => {
expect(sender).to.be(session);
expect(args).to.be.ok();
expect(args).to.eql({ oldValue: null, newValue: sender.kernel });
done();
});
session.initialize().catch(done);
Expand Down