Skip to content

Commit 6d69933

Browse files
author
Alan Fleming
committed
Improved manager logic and re-rendering when the kernel is re-connected.
1 parent ff90980 commit 6d69933

File tree

2 files changed

+107
-129
lines changed

2 files changed

+107
-129
lines changed

python/jupyterlab_widgets/src/manager.ts

Lines changed: 85 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import { IRenderMime } from '@jupyterlab/rendermime-interfaces';
2424

2525
import { ReadonlyPartialJSONValue } from '@lumino/coreutils';
2626

27-
import { INotebookModel } from '@jupyterlab/notebook';
27+
import { INotebookModel, NotebookModel } from '@jupyterlab/notebook';
2828

2929
import { IRenderMimeRegistry } from '@jupyterlab/rendermime';
3030

@@ -353,24 +353,20 @@ export abstract class LabWidgetManager
353353
* A singleton widget manager per kernel for the lifecycle of the kernel.
354354
* The factory of the rendermime will be update to use the widgetManager
355355
* directly if it isn't the globalRendermime.
356+
*
357+
* Note: The rendermime of the instance is always the global rendermime.
356358
*/
357359
export class KernelWidgetManager extends LabWidgetManager {
358360
constructor(
359361
kernel: Kernel.IKernelConnection,
360-
rendermime: IRenderMimeRegistry | null,
362+
rendermime?: IRenderMimeRegistry,
361363
pendingManagerMessage = 'Loading widget ...'
362364
) {
363-
if (!rendermime) {
364-
rendermime = LabWidgetManager.globalRendermime;
365-
}
366365
const instance = Private.kernelWidgetManagers.get(kernel.id);
367366
if (instance) {
368367
instance._useKernel(kernel);
369-
KernelWidgetManager.configureRendermime(
370-
rendermime,
371-
instance,
372-
pendingManagerMessage
373-
);
368+
configureRendermime(rendermime, instance, pendingManagerMessage);
369+
instance._firstLoad = false;
374370
return instance;
375371
}
376372
if (!kernel.handleComms) {
@@ -383,15 +379,11 @@ export class KernelWidgetManager extends LabWidgetManager {
383379
this.loadCustomWidgetDefinitions()
384380
);
385381
this._useKernel(kernel);
386-
KernelWidgetManager.configureRendermime(
387-
rendermime,
388-
this,
389-
pendingManagerMessage
390-
);
382+
configureRendermime(rendermime, this, pendingManagerMessage);
391383
}
392384

393385
_useKernel(this: KernelWidgetManager, kernel: Kernel.IKernelConnection) {
394-
if (!kernel.handleComms) {
386+
if (!kernel.handleComms || this._kernel === kernel) {
395387
return;
396388
}
397389
this._handleKernelChanged({
@@ -420,39 +412,6 @@ export class KernelWidgetManager extends LabWidgetManager {
420412
this.clear_state().then(() => this.restoreWidgets());
421413
}
422414

423-
/**
424-
* Configure a non-global rendermime. Passing the global rendermine will do
425-
* nothing.
426-
*
427-
* @param rendermime
428-
* @param manager The manager to use with WidgetRenderer.
429-
* @param pendingManagerMessage A message that is displayed while the manager
430-
* has not been provided. If manager is not provided here a non-empty string
431-
* assumes the manager will be provided at some time in the future.
432-
*
433-
* The default will search for a manager once.
434-
* @returns
435-
*/
436-
static configureRendermime(
437-
rendermime: IRenderMimeRegistry,
438-
manager?: KernelWidgetManager,
439-
pendingManagerMessage = ''
440-
) {
441-
if (rendermime === LabWidgetManager.globalRendermime) {
442-
return;
443-
}
444-
rendermime.removeMimeType(WIDGET_VIEW_MIMETYPE);
445-
rendermime.addFactory(
446-
{
447-
safe: false,
448-
mimeTypes: [WIDGET_VIEW_MIMETYPE],
449-
createRenderer: (options: IRenderMime.IRendererOptions) =>
450-
new WidgetRenderer(options, manager, pendingManagerMessage),
451-
},
452-
-10
453-
);
454-
}
455-
456415
_handleKernelConnectionStatusChange(
457416
sender: Kernel.IKernelConnection,
458417
status: Kernel.ConnectionStatus
@@ -482,6 +441,7 @@ export class KernelWidgetManager extends LabWidgetManager {
482441
break;
483442
}
484443
}
444+
485445
/**
486446
* Restore widgets from kernel.
487447
*/
@@ -503,7 +463,7 @@ export class KernelWidgetManager extends LabWidgetManager {
503463
return;
504464
}
505465
super.dispose();
506-
KernelWidgetManager.configureRendermime(this.rendermime);
466+
configureRendermime(this.rendermime);
507467
Private.kernelWidgetManagers.delete(this.kernel.id);
508468
this._handleKernelChanged({
509469
name: 'kernel',
@@ -518,6 +478,10 @@ export class KernelWidgetManager extends LabWidgetManager {
518478
return this._kernel;
519479
}
520480

481+
get firstLoad() {
482+
return this._firstLoad;
483+
}
484+
521485
loadCustomWidgetDefinitions() {
522486
for (const data of LabWidgetManager.WIDGET_REGISTRY) {
523487
this.register(data);
@@ -527,7 +491,7 @@ export class KernelWidgetManager extends LabWidgetManager {
527491
filterModelState(serialized_state: any): any {
528492
return this.filterExistingModelState(serialized_state);
529493
}
530-
494+
private _firstLoad = true;
531495
private _kernel: Kernel.IKernelConnection;
532496
protected _kernelRestoreInProgress = false;
533497
}
@@ -570,17 +534,7 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
570534
}
571535
});
572536
}
573-
if (rendermime !== LabWidgetManager.globalRendermime) {
574-
// Instruct the renderer to wait for the widgetManager.
575-
KernelWidgetManager.configureRendermime(
576-
rendermime,
577-
undefined,
578-
'Waiting for kernel'
579-
);
580-
}
581-
if (this.kernel) {
582-
this.updateWidgetManager();
583-
}
537+
this.updateWidgetManager();
584538
}
585539

586540
/**
@@ -605,38 +559,45 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
605559
}
606560

607561
async updateWidgetManager() {
562+
let wManager: KernelWidgetManager | undefined;
563+
if (!this.kernel) {
564+
//'No kernel' is matched in WidgetRenderer to supress 'model not found errors'.
565+
configureRendermime(this.rendermime, undefined, 'No kernel');
566+
} else {
567+
await this.context.sessionContext.ready;
568+
wManager = new KernelWidgetManager(this.kernel);
569+
}
570+
if (wManager === this._widgetManager) {
571+
return;
572+
}
608573
if (this._widgetManager) {
609574
this._widgetManager.onUnhandledIOPubMessage.disconnect(
610575
this.onUnhandledIOPubMessage,
611576
this
612577
);
613578
}
614-
if (this.kernel) {
615-
await this.context.sessionContext.ready;
616-
this._widgetManager = new KernelWidgetManager(this.kernel, null);
617-
this._widgetManager.onUnhandledIOPubMessage.connect(
618-
this.onUnhandledIOPubMessage,
619-
this
620-
);
621-
if (!this._widgetManager.restoredStatus) {
622-
await new Promise((resolve) => {
623-
this._widgetManager?.restored.connect(resolve);
624-
});
625-
if (!this.restored) {
626-
this.restoreWidgets(this._context!.model);
627-
}
579+
this._widgetManager = wManager;
580+
if (!wManager) {
581+
return;
582+
}
583+
if (wManager.firstLoad) {
584+
await new Promise((resolve) => {
585+
this._widgetManager?.restored.connect(resolve);
586+
});
587+
if (!this.restored) {
588+
this.restoreWidgets(this._context!.model);
628589
}
629-
KernelWidgetManager.configureRendermime(
630-
this.rendermime,
631-
this._widgetManager,
632-
'Loading widget ...'
633-
);
634-
if (this._renderers) {
635-
for (const r of this._renderers) {
636-
r.manager = this._widgetManager;
637-
}
590+
}
591+
if (this._renderers) {
592+
for (const r of this._renderers) {
593+
r.manager = wManager;
638594
}
639595
}
596+
configureRendermime(this.rendermime, wManager, 'Loading widget ...');
597+
wManager.onUnhandledIOPubMessage.connect(
598+
this.onUnhandledIOPubMessage,
599+
this
600+
);
640601
}
641602

642603
onUnhandledIOPubMessage(
@@ -676,7 +637,7 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
676637
this.setDirty();
677638
}
678639

679-
get widgetManager(): KernelWidgetManager | null {
640+
get widgetManager(): KernelWidgetManager | undefined {
680641
return this._widgetManager;
681642
}
682643

@@ -698,17 +659,13 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
698659
}
699660

700661
/**
701-
* Restore widgets from notebook and saved state.
662+
* Restore widgets from model.
702663
*/
703-
async restoreWidgets(
704-
notebook: INotebookModel,
705-
{ loadNotebook } = { loadNotebook: true }
706-
): Promise<void> {
664+
async restoreWidgets(model: INotebookModel): Promise<void> {
707665
try {
708-
if (loadNotebook) {
709-
await this._loadFromNotebook(notebook);
666+
if (model instanceof NotebookModel) {
667+
await this._loadFromNotebook(model);
710668
}
711-
712669
// If the restore worked above, then update our state.
713670
this._restoredStatus = true;
714671
this._restored.emit();
@@ -733,19 +690,6 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
733690
let state = widget_md[WIDGET_STATE_MIMETYPE];
734691
state = this.widgetManager.filterModelState(state);
735692

736-
// Ensure the kernel has been restored.
737-
let timeoutID;
738-
const restored = this.widgetManager.restored;
739-
if (!this.widgetManager.restoredStatus) {
740-
await new Promise((resolve, reject) => {
741-
restored.connect(resolve);
742-
timeoutID = window.setTimeout(
743-
() => reject('Timeout waiting for '),
744-
4000
745-
);
746-
});
747-
}
748-
clearTimeout(timeoutID);
749693
// Restore any widgets from saved state that are not live
750694
await this.widgetManager.set_state(state);
751695
}
@@ -812,7 +756,7 @@ export class WidgetManager extends Backbone.Model implements IDisposable {
812756
private _context: DocumentRegistry.IContext<INotebookModel>;
813757
private _rendermime: IRenderMimeRegistry;
814758
private _settings: WidgetManager.Settings;
815-
private _widgetManager: KernelWidgetManager | null;
759+
private _widgetManager: KernelWidgetManager | undefined;
816760
private _renderers: IterableIterator<WidgetRenderer> | undefined;
817761
}
818762

@@ -822,6 +766,39 @@ export namespace WidgetManager {
822766
};
823767
}
824768

769+
/**
770+
* Configure a non-global rendermime. Passing the global rendermine will do
771+
* nothing.
772+
*
773+
* @param rendermime
774+
* @param manager The manager to use with WidgetRenderer.
775+
* @param pendingManagerMessage A message that is displayed while the manager
776+
* has not been provided. If manager is not provided here a non-empty string
777+
* assumes the manager will be provided at some time in the future.
778+
*
779+
* The default will search for a manager once prior to waiting for a manager.
780+
* @returns
781+
*/
782+
function configureRendermime(
783+
rendermime?: IRenderMimeRegistry,
784+
manager?: KernelWidgetManager,
785+
pendingManagerMessage = ''
786+
) {
787+
if (!rendermime || rendermime === LabWidgetManager.globalRendermime) {
788+
return;
789+
}
790+
rendermime.removeMimeType(WIDGET_VIEW_MIMETYPE);
791+
rendermime.addFactory(
792+
{
793+
safe: false,
794+
mimeTypes: [WIDGET_VIEW_MIMETYPE],
795+
createRenderer: (options: IRenderMime.IRendererOptions) =>
796+
new WidgetRenderer(options, manager, pendingManagerMessage),
797+
},
798+
-10
799+
);
800+
}
801+
825802
/**
826803
* Get the widgetManager that owns the model.
827804
*/

0 commit comments

Comments
 (0)