Chain the kernel execution progress messages (#14689)

* Chain the kernel execution progress messages * Fix messages
microsoft · Nov 8, 2023 · d74c1e3 · d74c1e3
1 parent 1ad8860
commit d74c1e3
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 32 deletions.
diff --git a/src/kernels/api/kernel.ts b/src/kernels/api/kernel.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
-import { l10n, CancellationToken, Event, EventEmitter, ProgressLocation, extensions, window } from 'vscode';
+import { l10n, CancellationToken, Event, EventEmitter, ProgressLocation, extensions, window, Disposable } from 'vscode';
 import { ExecutionResult, Kernel } from '../../api';
 import { ServiceContainer } from '../../platform/ioc/container';
 import { IKernel, IKernelProvider, INotebookKernelExecution } from '../types';
@@ -14,50 +14,64 @@ import { Telemetry, sendTelemetryEvent } from '../../telemetry';
 import { StopWatch } from '../../platform/common/utils/stopWatch';
 import { Deferred, createDeferred, sleep } from '../../platform/common/utils/async';
 import { once } from '../../platform/common/utils/events';
+import { traceInfo, traceVerbose } from '../../platform/logging';
 
+function getExtensionDisplayName(extensionId: string) {
+    const extensionDisplayName = extensions.getExtension(extensionId)?.packageJSON.displayName;
+    return extensionDisplayName ? `${extensionDisplayName} (${extensionId})` : extensionId;
+}
 /**
  * Displays a progress indicator when 3rd party extensions execute code against a kernel.
  * We need this to notify users when execution takes place for:
  * 1. Transparency
  * 2. If users experience delays in kernel execution within notebooks, then they have an idea why this might be the case.
  */
 class KernelExecutionProgressIndicator {
-    private readonly extensionDisplayName: string;
     private readonly controllerDisplayName: string;
     private deferred?: Deferred<void>;
-    constructor(extensionId: string, kernel: IKernel) {
-        const extensionDisplayName = extensions.getExtension(extensionId)?.packageJSON.displayName;
-        this.extensionDisplayName = extensionDisplayName ? `${extensionDisplayName} (${extensionId})` : extensionId;
+    private disposable?: IDisposable;
+    private readonly title: string;
+    constructor(
+        private readonly extensionDisplayName: string,
+        kernel: IKernel
+    ) {
         this.controllerDisplayName = getDisplayNameOrNameOfKernelConnection(kernel.kernelConnectionMetadata);
+        this.title = l10n.t(
+            `Executing code against kernel '{0}' on behalf of the extension {1}`,
+            this.controllerDisplayName,
+            this.extensionDisplayName
+        );
     }
     dispose() {
-        this.hide();
+        this.disposable?.dispose();
     }
 
     show() {
         if (this.deferred && !this.deferred.completed) {
-            return;
+            const oldDeferred = this.deferred;
+            this.deferred = createDeferred<void>();
+            oldDeferred.resolve();
+            return (this.disposable = new Disposable(() => this.deferred?.resolve()));
         }
-        const deferred = (this.deferred = createDeferred<void>());
-        const title = l10n.t(
-            `Executing code against kernel '{0}' on behalf of the extension {1}`,
-            this.controllerDisplayName,
-            this.extensionDisplayName
-        );
-        // Give a grace period of 500ms to avoid too many progress indicators.
-        sleep(500)
-            .then(() => {
-                if (!deferred || deferred.completed) {
-                    return;
-                }
-                window
-                    .withProgress({ location: ProgressLocation.Notification, title }, async () => deferred.promise)
-                    .then(noop, noop);
-            })
-            .then(noop, noop);
+
+        this.showProgress().catch(noop);
+
+        this.deferred = createDeferred<void>();
+        return (this.disposable = new Disposable(() => this.deferred?.resolve()));
     }
-    hide() {
-        this.deferred?.resolve();
+    private async showProgress() {
+        // Give a grace period of 500ms to avoid too many progress indicators.
+        await sleep(500);
+        if (!this.deferred || this.deferred.completed) {
+            return;
+        }
+        await window.withProgress({ location: ProgressLocation.Notification, title: this.title }, async () => {
+            let deferred = this.deferred;
+            while (deferred && !deferred.completed) {
+                await deferred.promise;
+                deferred = this.deferred;
+            }
+        });
     }
 }
 
@@ -73,17 +87,21 @@ class WrappedKernelPerExtension implements Kernel {
         return this.kernel.onStatusChanged;
     }
 
+    private readonly extensionDisplayName: string;
     private readonly progress: KernelExecutionProgressIndicator;
+    private previousProgress?: IDisposable;
     constructor(
         private readonly extensionId: string,
         private readonly kernel: IKernel,
         private readonly execution: INotebookKernelExecution
     ) {
-        this.progress = new KernelExecutionProgressIndicator(extensionId, kernel);
+        this.extensionDisplayName = getExtensionDisplayName(extensionId);
+        this.progress = new KernelExecutionProgressIndicator(this.extensionDisplayName, kernel);
         once(kernel.onDisposed)(() => this.progress.dispose());
     }
 
     executeCode(code: string, token: CancellationToken): ExecutionResult {
+        this.previousProgress?.dispose();
         let completed = false;
         const measures = {
             requestHandledAfter: 0,
@@ -118,16 +136,17 @@ class WrappedKernelPerExtension implements Kernel {
             throw new Error('Kernel connection not available to execute 3rd party code');
         }
 
-        this.progress.show();
         const disposables: IDisposable[] = [];
         const onDidEmitOutput = new EventEmitter<{ mime: string; data: Uint8Array }[]>();
+        const progress = (this.previousProgress = this.progress.show());
+        disposables.push(progress);
         disposables.push(onDidEmitOutput);
         disposables.push({
             dispose: () => {
                 measures.duration = stopwatch.elapsedTime;
                 properties.mimeTypes = Array.from(mimeTypes).join(',');
                 completed = true;
-                this.progress.hide();
+                traceVerbose(`Kernel execution completed in ${measures.duration}ms, ${this.extensionDisplayName}.`);
             }
         });
         const request = executeSilentlyAndEmitOutput(this.kernel.session.kernel, code, (output) => {
@@ -157,6 +176,7 @@ class WrappedKernelPerExtension implements Kernel {
                 measures.interruptedAfter = stopwatch.elapsedTime;
                 properties.interruptedBeforeHandled = !properties.requestHandled;
                 if (properties.requestHandled) {
+                    traceInfo(`Kernel Interrupted by extension ${this.extensionDisplayName}`);
                     this.kernel.interrupt().catch(() => request.dispose());
                 } else {
                     request.dispose();

diff --git a/src/kernels/kernelStatusProvider.ts b/src/kernels/kernelStatusProvider.ts
@@ -66,7 +66,9 @@ export class KernelStatusProvider implements IExtensionSyncActivationService {
             async () => {
                 const progress = KernelProgressReporter.createProgressReporter(
                     kernel.resourceUri,
-                    DataScience.interruptKernelStatus
+                    DataScience.interruptKernelStatus(
+                        getDisplayNameOrNameOfKernelConnection(kernel.kernelConnectionMetadata)
+                    )
                 );
                 this.interruptProgress.set(kernel, progress);
             },

diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts
@@ -369,7 +369,7 @@ export namespace DataScience {
             if (numberOfConnections === 1) {
                 return l10n.t('1 connection');
             }
-            return l10n.t('{1} connections', numberOfConnections.toString());
+            return l10n.t('{0} connections', numberOfConnections.toString());
         }
         if (numberOfConnections === 0) {
             return l10n.t('Last activity {0}', fromNow(time, true, false, false));
@@ -447,7 +447,8 @@ export namespace DataScience {
             "'Kernelspec' module not installed in the selected interpreter ({0}).\n Please re-install or update 'jupyter'.",
             pythonExecFileName
         );
-    export const interruptKernelStatus = l10n.t('Interrupting Jupyter Kernel');
+    export const interruptKernelStatus = (kernelDisplayName: string) =>
+        l10n.t('Interrupting Kernel {0}', kernelDisplayName);
     export const exportPythonQuickPickLabel = l10n.t('Python Script');
     export const exportHTMLQuickPickLabel = l10n.t('HTML');
     export const exportPDFQuickPickLabel = l10n.t('PDF');