Skip to content

Commit 86b5b0e

Browse files
author
Alan Fleming
committed
Improved KernelWidgetManager creation and kernel restoration.
1 parent 79d6a0c commit 86b5b0e

File tree

4 files changed

+72
-96
lines changed

4 files changed

+72
-96
lines changed

python/jupyterlab_widgets/src/manager.ts

+66-71
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,7 @@ export const WIDGET_STATE_MIMETYPE =
6666
/**
6767
* A widget manager that returns Lumino widgets.
6868
*/
69-
export abstract class LabWidgetManager
70-
extends ManagerBase
71-
implements IDisposable
72-
{
73-
constructor(rendermime: IRenderMimeRegistry) {
74-
super();
75-
this._rendermime = rendermime;
76-
}
77-
69+
abstract class LabWidgetManager extends ManagerBase implements IDisposable {
7870
/**
7971
* Default callback handler to emit unhandled kernel messages.
8072
*/
@@ -181,7 +173,6 @@ export abstract class LabWidgetManager
181173
return;
182174
}
183175
this._isDisposed = true;
184-
this._rendermime = null!;
185176

186177
if (this._commRegistration) {
187178
this._commRegistration.dispose();
@@ -245,10 +236,6 @@ export abstract class LabWidgetManager
245236

246237
abstract get kernel(): Kernel.IKernelConnection | null;
247238

248-
get rendermime(): IRenderMimeRegistry {
249-
return this._rendermime;
250-
}
251-
252239
/**
253240
* A signal emitted when state is restored to the widget manager.
254241
*
@@ -298,6 +285,7 @@ export abstract class LabWidgetManager
298285
* @return Promise that resolves when the widget state is cleared.
299286
*/
300287
async clear_state(): Promise<void> {
288+
this._restoredStatus = false;
301289
await super.clear_state();
302290
this._modelsSync = new Map();
303291
}
@@ -331,15 +319,13 @@ export abstract class LabWidgetManager
331319
await this.handle_comm_open(oldComm, msg);
332320
};
333321

334-
static globalRendermime: IRenderMimeRegistry;
322+
static rendermime: IRenderMimeRegistry;
335323

336324
protected _restored = new Signal<this, void>(this);
337325
protected _restoredStatus = false;
338-
protected _kernelRestoreInProgress = false;
339326

340327
private _isDisposed = false;
341328
private _registry: SemVerCache<ExportData> = new SemVerCache<ExportData>();
342-
private _rendermime: IRenderMimeRegistry;
343329

344330
private _commRegistration: IDisposable;
345331

@@ -352,46 +338,31 @@ export abstract class LabWidgetManager
352338
}
353339

354340
/**
355-
* A singleton widget manager per kernel for the lifecycle of the kernel.
356-
* The factory of the rendermime will be updated to use the widgetManager
357-
* directly if it isn't the globalRendermime.
358-
*
359-
* Note: The rendermime of the instance is always the global rendermime.
341+
* KernelWidgetManager is singleton widget manager per kernel.id.
342+
* This class should not be created directly or subclassed, instead use
343+
* the class method `KernelWidgetManager.getManager(kernel)`.
360344
*/
361345
export class KernelWidgetManager extends LabWidgetManager {
362-
constructor(
363-
kernel: Kernel.IKernelConnection,
364-
rendermime?: IRenderMimeRegistry,
365-
pendingManagerMessage = 'Loading widget ...'
366-
) {
367-
const instance = Private.managers.get(kernel.id);
368-
if (instance) {
369-
instance._useKernel(kernel);
370-
KernelWidgetManager.configureRendermime(
371-
rendermime,
372-
instance,
373-
pendingManagerMessage
374-
);
375-
return instance;
346+
constructor(kernel: Kernel.IKernelConnection) {
347+
if (Private.managers.has(kernel.id)) {
348+
throw new Error('A manager already exists!');
376349
}
377350
if (!kernel.handleComms) {
378351
throw new Error('Kernel does not have handleComms enabled');
379352
}
380-
super(LabWidgetManager.globalRendermime);
353+
super();
381354
Private.managers.set(kernel.id, this);
382355
this.loadCustomWidgetDefinitions();
383356
LabWidgetManager.WIDGET_REGISTRY.changed.connect(() =>
384357
this.loadCustomWidgetDefinitions()
385358
);
386-
this._useKernel(kernel);
387-
KernelWidgetManager.configureRendermime(
388-
rendermime,
389-
this,
390-
pendingManagerMessage
391-
);
359+
this._updateKernel(kernel);
392360
}
393361

394-
_useKernel(this: KernelWidgetManager, kernel: Kernel.IKernelConnection) {
362+
private _updateKernel(
363+
this: KernelWidgetManager,
364+
kernel: Kernel.IKernelConnection
365+
) {
395366
if (!kernel.handleComms || this._kernel === kernel) {
396367
return;
397368
}
@@ -409,13 +380,15 @@ export class KernelWidgetManager extends LabWidgetManager {
409380
this._handleKernelConnectionStatusChange,
410381
this
411382
);
383+
this._kernel.disposed.disconnect(this._onKernelDisposed, this);
412384
}
413385
this._kernel = kernel;
414-
this._kernel.statusChanged.connect(this._handleKernelStatusChange, this);
415-
this._kernel.connectionStatusChanged.connect(
386+
kernel.statusChanged.connect(this._handleKernelStatusChange, this);
387+
kernel.connectionStatusChanged.connect(
416388
this._handleKernelConnectionStatusChange,
417389
this
418390
);
391+
kernel.disposed.connect(this._onKernelDisposed, this);
419392
this.restoreWidgets();
420393
}
421394

@@ -437,7 +410,7 @@ export class KernelWidgetManager extends LabWidgetManager {
437410
manager?: KernelWidgetManager,
438411
pendingManagerMessage = ''
439412
) {
440-
if (!rendermime || rendermime === LabWidgetManager.globalRendermime) {
413+
if (!rendermime || rendermime === LabWidgetManager.rendermime) {
441414
return;
442415
}
443416
rendermime.removeMimeType(WIDGET_VIEW_MIMETYPE);
@@ -457,27 +430,18 @@ export class KernelWidgetManager extends LabWidgetManager {
457430
): void {
458431
switch (status) {
459432
case 'connected':
460-
// Only restore if we aren't currently trying to restore from the kernel
461-
// (for example, in our initial restore from the constructor).
462-
if (!this._kernelRestoreInProgress) {
463-
this.restoreWidgets();
464-
}
433+
this.restoreWidgets();
465434
break;
466435
case 'disconnected':
467436
this.disconnect();
468437
break;
469438
}
470439
}
471440

472-
static existsWithActiveKenel(id: string) {
473-
const widgetManager = Private.managers.get(id);
474-
return widgetManager?._restoredStatus;
475-
}
476-
477441
/**
478-
* Get the KernelWidgetManager that owns the model.
442+
* Find the KernelWidgetManager that owns the model.
479443
*/
480-
static async getManager(
444+
static async findManager(
481445
model_id: string,
482446
delays = [100, 1000]
483447
): Promise<KernelWidgetManager> {
@@ -494,6 +458,28 @@ export class KernelWidgetManager extends LabWidgetManager {
494458
);
495459
}
496460

461+
/**
462+
* The correct way to get a KernelWidgetManager
463+
* @param kernel IKernelConnection
464+
* @returns
465+
*/
466+
static async getManager(
467+
kernel: Kernel.IKernelConnection
468+
): Promise<KernelWidgetManager> {
469+
let manager = Private.managers.get(kernel.id);
470+
if (!manager) {
471+
manager = new KernelWidgetManager(kernel);
472+
}
473+
if (kernel.handleComms) {
474+
manager._updateKernel(kernel);
475+
if (!manager.restoredStatus) {
476+
const restored = manager.restored;
477+
await new Promise((resolve) => restored.connect(resolve));
478+
}
479+
}
480+
return manager;
481+
}
482+
497483
_handleKernelStatusChange(
498484
sender: Kernel.IKernelConnection,
499485
status: Kernel.Status
@@ -506,23 +492,36 @@ export class KernelWidgetManager extends LabWidgetManager {
506492
}
507493
}
508494

495+
async _onKernelDisposed() {
496+
const model = await KernelWidgetManager.kernels.findById(this.kernel?.id);
497+
if (model) {
498+
const kernel = KernelWidgetManager.kernels.connectTo({ model });
499+
this._updateKernel(kernel);
500+
}
501+
}
502+
509503
/**
510504
* Restore widgets from kernel.
511505
*/
512506
async restoreWidgets(): Promise<void> {
507+
if (this._kernelRestoreInProgress) {
508+
return;
509+
}
513510
this._restoredStatus = false;
514511
this._kernelRestoreInProgress = true;
515512
try {
516513
await this.clear_state();
517514
await this._loadFromKernel();
518-
} catch (err) {
519-
// Do nothing
515+
} catch {
516+
/* empty */
517+
} finally {
518+
this._restoredStatus = true;
519+
this._kernelRestoreInProgress = false;
520+
this.triggerRestored();
520521
}
521-
this.triggerRestored();
522522
}
523523

524524
triggerRestored() {
525-
this._restoredStatus = true;
526525
this._restored.emit();
527526
}
528527
/**
@@ -533,7 +532,6 @@ export class KernelWidgetManager extends LabWidgetManager {
533532
return;
534533
}
535534
super.dispose();
536-
KernelWidgetManager.configureRendermime(this.rendermime);
537535
Private.managers.delete(this.kernel.id);
538536
this._handleKernelChanged({
539537
name: 'kernel',
@@ -557,9 +555,9 @@ export class KernelWidgetManager extends LabWidgetManager {
557555
filterModelState(serialized_state: any): any {
558556
return this.filterExistingModelState(serialized_state);
559557
}
560-
558+
static kernels: Kernel.IManager;
561559
private _kernel: Kernel.IKernelConnection;
562-
protected _kernelRestoreInProgress = false;
560+
private _kernelRestoreInProgress = false;
563561
}
564562

565563
/**
@@ -630,7 +628,7 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
630628
rendermime: IRenderMimeRegistry,
631629
manager: WidgetManager
632630
) {
633-
if (rendermime === LabWidgetManager.globalRendermime) {
631+
if (rendermime === LabWidgetManager.rendermime) {
634632
return;
635633
}
636634
rendermime.removeMimeType(WIDGET_VIEW_MIMETYPE);
@@ -648,7 +646,7 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
648646
let wManager: KernelWidgetManager | undefined;
649647
await this.context.sessionContext.ready;
650648
if (this.kernel) {
651-
wManager = new KernelWidgetManager(this.kernel);
649+
wManager = await KernelWidgetManager.getManager(this.kernel);
652650
}
653651
if (wManager === this._widgetManager) {
654652
return;
@@ -796,7 +794,6 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
796794
this._renderers = null!;
797795
this._context = null!;
798796
this._context = null!;
799-
this._rendermime = null!;
800797
this._settings = null!;
801798
}
802799

@@ -831,8 +828,6 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
831828
}
832829
}
833830
static loggerRegistry: ILoggerRegistry | null;
834-
// protected _restored = new Signal<this, void>(this);
835-
// protected _restoredStatus = false;
836831
private _isDisposed = false;
837832
private _context: DocumentRegistry.Context;
838833
private _rendermime: IRenderMimeRegistry;

python/jupyterlab_widgets/src/output.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { Panel } from '@lumino/widgets';
99

1010
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
1111

12-
import { LabWidgetManager } from './manager';
12+
import { KernelWidgetManager } from './manager';
1313

1414
import { OutputAreaModel, OutputArea } from '@jupyterlab/outputarea';
1515

@@ -44,7 +44,7 @@ export class OutputModel extends outputBase.OutputModel {
4444
* Reset the message id.
4545
*/
4646
reset_msg_id(): void {
47-
const kernel = this.widget_manager.kernel;
47+
const kernel = (this.widget_manager as KernelWidgetManager).kernel;
4848
const msgId = this.get('msg_id');
4949
const oldMsgId = this.previous('msg_id');
5050

@@ -98,8 +98,6 @@ export class OutputModel extends outputBase.OutputModel {
9898
}
9999
}
100100

101-
widget_manager: LabWidgetManager;
102-
103101
private _msgHook: (msg: KernelMessage.IIOPubMessage) => boolean;
104102
private _outputs: OutputAreaModel;
105103
static rendermime: IRenderMimeRegistry;

python/jupyterlab_widgets/src/plugin.ts

+3-20
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import { WidgetRenderer } from './renderer';
2626

2727
import {
2828
KernelWidgetManager,
29-
LabWidgetManager,
3029
WIDGET_VIEW_MIMETYPE,
3130
WidgetManager,
3231
} from './manager';
@@ -103,23 +102,7 @@ function activateWidgetExtension(
103102
): base.IJupyterWidgetRegistry {
104103
const { commands } = app;
105104
const trans = (translator ?? nullTranslator).load('jupyterlab_widgets');
106-
107-
app.serviceManager.kernels.runningChanged.connect((models) => {
108-
for (const model of models.running()) {
109-
if (
110-
model &&
111-
model.name === 'python3' &&
112-
model.execution_state !== 'starting' &&
113-
!KernelWidgetManager.existsWithActiveKenel(model.id)
114-
) {
115-
const kernel = app.serviceManager.kernels.connectTo({ model: model });
116-
if (kernel.handleComms) {
117-
new KernelWidgetManager(kernel);
118-
}
119-
}
120-
}
121-
});
122-
105+
KernelWidgetManager.kernels = app.serviceManager.kernels;
123106
if (settingRegistry !== null) {
124107
settingRegistry
125108
.load(managerPlugin.id)
@@ -132,7 +115,7 @@ function activateWidgetExtension(
132115
});
133116
}
134117
WidgetManager.loggerRegistry = loggerRegistry;
135-
LabWidgetManager.globalRendermime = rendermime;
118+
KernelWidgetManager.rendermime = rendermime;
136119
// Add a default widget renderer.
137120
rendermime.addFactory(
138121
{
@@ -175,7 +158,7 @@ function activateWidgetExtension(
175158

176159
return {
177160
registerWidget(data: base.IWidgetRegistryData): void {
178-
LabWidgetManager.WIDGET_REGISTRY.push(data);
161+
KernelWidgetManager.WIDGET_REGISTRY.push(data);
179162
},
180163
};
181164
}

python/jupyterlab_widgets/src/renderer.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export class WidgetRenderer
7272
}
7373
if (!this._pendingManagerMessage && !this._managerIsSet) {
7474
try {
75-
this.manager = await KernelWidgetManager.getManager(source.model_id);
75+
this.manager = await KernelWidgetManager.findManager(source.model_id);
7676
} catch {
7777
this.node.textContent = `KernelWidgetManager not found for model: ${model.data['text/plain']}`;
7878
return;

0 commit comments

Comments
 (0)