Updates to the kernel panel of the "running" sidebar (jupyterlab#13792)

* Only query the API once per render of running items * Fix caching logic * Fix typo in docstrings * Expose the service manager's internal kernel manager instance to extension authors * Phosphor => Lumino * Use both session and kernel managers in running panel * Incremental update, render only kernels instead of sessions * Reimplement shutdown * Minimum viable implementation of open() * Fix KernelManager identity check * minimum viable running kernels panel * Fix kernel shutdown all * Simplify the label title for running kernels * tegridy * Update Playwright Snapshots * Improve UI tests robustness * lint * Update Playwright Snapshots * Update snapshot Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Frédéric Collonval <[email protected]> Co-authored-by: Frédéric Collonval <[email protected]>
skyetim · Jan 24, 2023 · 40afbd9 · 40afbd9
1 parent 2ec40f8
commit 40afbd9
Show file tree

Hide file tree

Showing 14 changed files with 163 additions and 83 deletions.
diff --git a/galata/test/documentation/general.test.ts b/galata/test/documentation/general.test.ts
@@ -141,6 +141,7 @@ test.describe('General', () => {
 
     await page.click('[title="Running Terminals and Kernels"]');
 
+    await page.locator('text="Python 3 (ipykernel) {1}"').waitFor();
     expect(
       await page.screenshot({ clip: { y: 27, x: 0, width: 283, height: 400 } })
     ).toMatchSnapshot('interface_tabs.png');
@@ -460,6 +461,10 @@ test.describe('General', () => {
 
     await page.click('[title="Running Terminals and Kernels"]');
 
+    await expect(page.locator('text="Python 3 (ipykernel) {1}"')).toHaveCount(
+      2
+    );
+
     expect(
       await page.screenshot({ clip: { y: 27, x: 0, width: 283, height: 400 } })
     ).toMatchSnapshot('running_layout.png');

diff --git a/.../documentation/general.test.ts-snapshots/interface-tabs-documentation-linux.png b/.../documentation/general.test.ts-snapshots/interface-tabs-documentation-linux.png
diff --git a/.../documentation/general.test.ts-snapshots/running-layout-documentation-linux.png b/.../documentation/general.test.ts-snapshots/running-layout-documentation-linux.png
diff --git a/packages/application/style/scrollbar.css b/packages/application/style/scrollbar.css
@@ -116,7 +116,7 @@
 }
 
 /*
- * Phosphor
+ * Lumino
  */
 
 .lm-ScrollBar[data-orientation='horizontal'] {

diff --git a/packages/running-extension/package.json b/packages/running-extension/package.json
@@ -38,12 +38,14 @@
   },
   "dependencies": {
     "@jupyterlab/application": "^4.0.0-alpha.17",
-    "@jupyterlab/coreutils": "^6.0.0-alpha.17",
     "@jupyterlab/docregistry": "^4.0.0-alpha.17",
+    "@jupyterlab/rendermime-interfaces": "^3.8.0-alpha.17",
     "@jupyterlab/running": "^4.0.0-alpha.17",
     "@jupyterlab/services": "^7.0.0-alpha.17",
     "@jupyterlab/translation": "^4.0.0-alpha.17",
     "@jupyterlab/ui-components": "^4.0.0-alpha.32",
+    "@lumino/commands": "^2.0.0-alpha.6",
+    "@lumino/polling": "^2.0.0-alpha.6",
     "@lumino/signaling": "^2.0.0-alpha.6",
     "@lumino/widgets": "^2.0.0-alpha.6"
   },

diff --git a/packages/running-extension/src/index.ts b/packages/running-extension/src/index.ts
@@ -72,7 +72,7 @@ function activate(
   if (labShell) {
     addOpenTabsSessionManager(runningSessionManagers, translator, labShell);
   }
-  addKernelRunningSessionManager(runningSessionManagers, translator, app);
+  void addKernelRunningSessionManager(runningSessionManagers, translator, app);
   // Rank has been chosen somewhat arbitrarily to give priority to the running
   // sessions widget in the sidebar.
   app.shell.add(running, 'left', { rank: 200, type: 'Sessions and Tabs' });

diff --git a/packages/running-extension/src/kernels.ts b/packages/running-extension/src/kernels.ts
@@ -2,83 +2,138 @@
 // Distributed under the terms of the Modified BSD License.
 
 import { JupyterFrontEnd } from '@jupyterlab/application';
-import { PathExt } from '@jupyterlab/coreutils';
+import { IRenderMime } from '@jupyterlab/rendermime-interfaces';
 import { IRunningSessionManagers, IRunningSessions } from '@jupyterlab/running';
-import { Session } from '@jupyterlab/services';
+import { Kernel, KernelSpec, Session } from '@jupyterlab/services';
 import { ITranslator } from '@jupyterlab/translation';
-import { consoleIcon, fileIcon, notebookIcon } from '@jupyterlab/ui-components';
+import { jupyterIcon, LabIcon } from '@jupyterlab/ui-components';
+import { CommandRegistry } from '@lumino/commands';
+import { Throttler } from '@lumino/polling';
+import { Signal } from '@lumino/signaling';
 
 /**
  * Add the running kernel manager (notebooks & consoles) to the running panel.
  */
-export function addKernelRunningSessionManager(
+export async function addKernelRunningSessionManager(
   managers: IRunningSessionManagers,
   translator: ITranslator,
   app: JupyterFrontEnd
-): void {
+): Promise<void> {
+  const { commands } = app;
   const trans = translator.load('jupyterlab');
-  const manager = app.serviceManager.sessions;
-  const specsManager = app.serviceManager.kernelspecs;
-  function filterSessions(m: Session.IModel) {
-    return !!(
-      (m.name || PathExt.basename(m.path)).indexOf('.') !== -1 || m.name
-    );
-  }
+  const { kernels, kernelspecs, sessions } = app.serviceManager;
+  const { runningChanged, RunningKernel } = Private;
+  const throttler = new Throttler(() => runningChanged.emit(undefined), 100);
+
+  // Throttle signal emissions from the kernel and session managers.
+  kernels.runningChanged.connect(() => void throttler.invoke());
+  sessions.runningChanged.connect(() => void throttler.invoke());
+
+  await Promise.all([kernelspecs.ready, kernels.ready]);
+
+  managers.add({
+    name: trans.__('Kernels'),
+    running: () =>
+      Array.from(kernels.running()).map(
+        kernel =>
+          new RunningKernel({
+            commands,
+            kernel,
+            kernels,
+            sessions,
+            spec: kernelspecs.specs?.kernelspecs[kernel.name],
+            trans
+          })
+      ),
+    shutdownAll: () => kernels.shutdownAll(),
+    refreshRunning: () =>
+      Promise.all([kernels.refreshRunning(), sessions.refreshRunning()]),
+    runningChanged,
+    shutdownLabel: trans.__('Shut Down'),
+    shutdownAllLabel: trans.__('Shut Down All'),
+    shutdownAllConfirmationText: trans.__(
+      'Are you sure you want to permanently shut down all running kernels?'
+    )
+  });
+}
 
-  class RunningKernel implements IRunningSessions.IRunningItem {
-    constructor(model: Session.IModel) {
-      this._model = model;
+namespace Private {
+  export class RunningKernel implements IRunningSessions.IRunningItem {
+    constructor(options: RunningKernel.IOptions) {
+      this.commands = options.commands;
+      this.kernel = options.kernel;
+      this.kernels = options.kernels;
+      this.sessions = options.sessions;
+      this.spec = options.spec || null;
+      this.trans = options.trans;
+      this._icon = options.icon || jupyterIcon;
     }
+
+    readonly commands: CommandRegistry;
+
+    readonly kernel: Kernel.IModel;
+
+    readonly kernels: Kernel.IManager;
+
+    readonly sessions: Session.IManager;
+
+    readonly spec: KernelSpec.ISpecModel | null;
+
+    readonly trans: IRenderMime.TranslationBundle;
+
     open() {
-      const { path, type } = this._model;
-      if (type.toLowerCase() === 'console') {
-        void app.commands.execute('console:open', { path });
-      } else {
-        void app.commands.execute('docmanager:open', { path });
+      for (const session of this.sessions.running()) {
+        if (this.kernel.id !== session.kernel?.id) {
+          continue;
+        }
+        const { path, type } = session;
+        const command = type === 'console' ? 'console:open' : 'docmanager:open';
+        void this.commands.execute(command, { path });
       }
     }
+
     shutdown() {
-      return manager.shutdown(this._model.id);
+      return this.kernels.shutdown(this.kernel.id);
     }
+
     icon() {
-      const { name, path, type } = this._model;
-      if ((name || PathExt.basename(path)).indexOf('.ipynb') !== -1) {
-        return notebookIcon;
-      } else if (type.toLowerCase() === 'console') {
-        return consoleIcon;
-      }
-      return fileIcon;
+      // TODO: Use the icon from `this.spec.resources` instead.
+      return this._icon;
     }
+
     label() {
-      return this._model.name || PathExt.basename(this._model.path);
+      const { kernel, spec } = this;
+      const name = spec?.display_name || kernel.name;
+      return `${name} {${kernel.connections ?? '-'}}`;
     }
+
     labelTitle() {
-      const { kernel, path } = this._model;
-      let kernelName = kernel?.name;
-      if (kernelName && specsManager.specs) {
-        const spec = specsManager.specs.kernelspecs[kernelName];
-        kernelName = spec ? spec.display_name : 'unknown';
+      const { trans } = this;
+      const { id } = this.kernel;
+      const title = [`${this.label()}: ${id}`];
+      for (const session of this.sessions.running()) {
+        if (this.kernel.id === session.kernel?.id) {
+          const { path, type } = session;
+          title.push(trans.__(`%1\nPath: %2`, type, path));
+        }
       }
-      return trans.__('Path: %1\nKernel: %2', path, kernelName);
+      return title.join('\n\n');
     }
 
-    private _model: Session.IModel;
+    private _icon: LabIcon;
   }
 
-  managers.add({
-    name: trans.__('Kernels'),
-    running: () => {
-      return Array.from(manager.running())
-        .filter(filterSessions)
-        .map(model => new RunningKernel(model));
-    },
-    shutdownAll: () => manager.shutdownAll(),
-    refreshRunning: () => manager.refreshRunning(),
-    runningChanged: manager.runningChanged,
-    shutdownLabel: trans.__('Shut Down'),
-    shutdownAllLabel: trans.__('Shut Down All'),
-    shutdownAllConfirmationText: trans.__(
-      'Are you sure you want to permanently shut down all running kernels?'
-    )
-  });
+  export namespace RunningKernel {
+    export interface IOptions {
+      commands: CommandRegistry;
+      icon?: LabIcon;
+      kernel: Kernel.IModel;
+      kernels: Kernel.IManager;
+      sessions: Session.IManager;
+      spec?: KernelSpec.ISpecModel;
+      trans: IRenderMime.TranslationBundle;
+    }
+  }
+
+  export const runningChanged = new Signal<unknown, unknown>({});
 }
diff --git a/packages/running-extension/tsconfig.json b/packages/running-extension/tsconfig.json
@@ -10,10 +10,10 @@
       "path": "../application"
     },
     {
-      "path": "../coreutils"
+      "path": "../docregistry"
     },
     {
-      "path": "../docregistry"
+      "path": "../rendermime-interfaces"
     },
     {
       "path": "../running"

diff --git a/packages/running/src/index.tsx b/packages/running/src/index.tsx
@@ -232,7 +232,9 @@ class Section extends PanelWithToolbar {
       });
     }
 
-    const enabled = options.manager.running().length > 0;
+    let runningItems = options.manager.running();
+    let cached = true;
+    const enabled = runningItems.length > 0;
     this._button = new ToolbarButton({
       label: shutdownAllLabel,
       className: `${SHUTDOWN_ALL_BUTTON_CLASS} jp-mod-styled ${
@@ -249,10 +251,17 @@ class Section extends PanelWithToolbar {
       ReactWidget.create(
         <UseSignal signal={options.manager.runningChanged}>
           {() => {
+            // Cache the running items for the intial load and request from
+            // the service every subsequent load.
+            if (cached) {
+              cached = false;
+            } else {
+              runningItems = options.manager.running();
+            }
             return (
               <div className={CONTAINER_CLASS}>
                 <List
-                  runningItems={options.manager.running()}
+                  runningItems={runningItems}
                   shutdownLabel={options.manager.shutdownLabel}
                   shutdownAllLabel={shutdownAllLabel}
                   shutdownItemIcon={options.manager.shutdownItemIcon}
@@ -278,15 +287,12 @@ class Section extends PanelWithToolbar {
   }
 
   private _updateButton(): void {
-    this._button.enabled = this._manager.running().length > 0;
-    if (this._button.enabled) {
-      this._button.node
-        .querySelector('button')
-        ?.classList.remove('jp-mod-disabled');
+    const button = this._button;
+    button.enabled = this._manager.running().length > 0;
+    if (button.enabled) {
+      button.node.querySelector('button')?.classList.remove('jp-mod-disabled');
     } else {
-      this._button.node
-        .querySelector('button')
-        ?.classList.add('jp-mod-disabled');
+      button.node.querySelector('button')?.classList.add('jp-mod-disabled');
     }
   }
 

diff --git a/packages/services/src/kernel/manager.ts b/packages/services/src/kernel/manager.ts
@@ -261,7 +261,14 @@ export class KernelManager extends BaseManager implements Kernel.IManager {
         if (!existing) {
           return false;
         }
-        return existing.name === model.name;
+        return (
+          existing.connections === model.connections &&
+          existing.execution_state === model.execution_state &&
+          existing.last_activity === model.last_activity &&
+          existing.name === model.name &&
+          existing.reason === model.reason &&
+          existing.traceback === model.traceback
+        );
       })
     ) {
       // Identical models list (presuming models does not contain duplicate

diff --git a/packages/services/src/manager.ts b/packages/services/src/manager.ts
@@ -45,15 +45,15 @@ export class ServiceManager implements ServiceManager.IManager {
     const standby = options.standby ?? 'when-hidden';
     const normalized = { defaultDrive, serverSettings, standby };
 
-    const kernelManager = options.kernels || new KernelManager(normalized);
     this.serverSettings = serverSettings;
     this.contents = options.contents || new ContentsManager(normalized);
     this.events = options.events || new EventManager(normalized);
+    this.kernels = options.kernels || new KernelManager(normalized);
     this.sessions =
       options.sessions ||
       new SessionManager({
         ...normalized,
-        kernelManager: kernelManager
+        kernelManager: this.kernels
       });
     this.settings = options.settings || new SettingManager(normalized);
     this.terminals = options.terminals || new TerminalManager(normalized);
@@ -120,7 +120,12 @@ export class ServiceManager implements ServiceManager.IManager {
   readonly sessions: Session.IManager;
 
   /**
-   * Get the session manager instance.
+   * Get the kernel manager instance.
+   */
+  readonly kernels: Kernel.IManager;
+
+  /**
+   * Get the kernelspec manager instance.
    */
   readonly kernelspecs: KernelSpec.IManager;
 
@@ -216,11 +221,6 @@ export namespace ServiceManager {
    * The options used to create a service manager.
    */
   export interface IOptions extends IManagers {
-    /**
-     * Kernel manager of the manager.
-     */
-    readonly kernels: Kernel.IManager;
-
     /**
      * The default drive for the contents manager.
      */
@@ -269,7 +269,12 @@ export namespace ServiceManager {
     readonly sessions: Session.IManager;
 
     /**
-     * The session manager for the manager.
+     * The kernel manager of the manager.
+     */
+    readonly kernels: Kernel.IManager;
+
+    /**
+     * The kernelspec manager for the manager.
      */
     readonly kernelspecs: KernelSpec.IManager;
 

diff --git a/packages/ui-components/src/components/menu.ts b/packages/ui-components/src/components/menu.ts
@@ -160,7 +160,7 @@ export class RankedMenu extends Menu implements IRankedMenu {
     const added: IDisposableMenuItem[] = [];
 
     // Insert a separator before the group.
-    // Phosphor takes care of superfluous leading,
+    // Lumino takes care of superfluous leading,
     // trailing, and duplicate separators.
     if (this._includeSeparators) {
       added.push(

diff --git a/packages/ui-components/src/components/toolbar.tsx b/packages/ui-components/src/components/toolbar.tsx
@@ -709,7 +709,7 @@ export function addToolbarButtonClass(w: Widget): Widget {
 }
 
 /**
- * Phosphor Widget version of static ToolbarButtonComponent.
+ * Lumino Widget version of static ToolbarButtonComponent.
  */
 export class ToolbarButton extends ReactWidget {
   /**
-Original file line number
+Diff line change
@@ Expand Up / @@ -116,7 +116,7 @@ @@
     }
     /*
-     * Phosphor
+     * Lumino
      */
     .lm-ScrollBar[data-orientation='horizontal'] {
@@ Expand Down @@