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

Chain the kernel execution progress messages #14689

Merged
merged 2 commits into from
Nov 8, 2023
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
78 changes: 49 additions & 29 deletions src/kernels/api/kernel.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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;
}
});
}
}

Expand All @@ -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,
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 3 additions & 1 deletion src/kernels/kernelStatusProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
},
Expand Down
5 changes: 3 additions & 2 deletions src/platform/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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');
Expand Down
Loading