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.

Sign up for GitHub

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

Better indication of kernel having pending cells #7808

Closed
wants to merge 2 commits into from
Closed

Better indication of kernel having pending cells #7808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8d87c0
Better indication of kernel having pending cells
DonJayamanne Oct 4, 2021
d5cedec
misc
DonJayamanne Oct 5, 2021
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
77 changes: 28 additions & 49 deletions src/client/datascience/commands/activeEditorContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { IDisposable, IDisposableRegistry } from '../../common/types';
import { isNotebookCell } from '../../common/utils/misc';
import { EditorContexts } from '../constants';
import { getActiveInteractiveWindow } from '../interactive-window/helpers';
import { IKernelProvider } from '../jupyter/kernels/types';
import { InteractiveWindowView, JupyterNotebookView } from '../notebook/constants';
import { getNotebookMetadata, isPythonNotebook } from '../notebook/helpers/helpers';
import { IInteractiveWindow, IInteractiveWindowProvider, INotebook, INotebookProvider } from '../types';
Expand Down Expand Up @@ -42,7 +43,8 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
@inject(ICommandManager) private readonly commandManager: ICommandManager,
@inject(IDisposableRegistry) disposables: IDisposableRegistry,
@inject(INotebookProvider) private readonly notebookProvider: INotebookProvider,
@inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook
@inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook,
@inject(IKernelProvider) private readonly kernelProvider: IKernelProvider
) {
disposables.push(this);
this.nativeContext = new ContextKey(EditorContexts.IsNativeActive, this.commandManager);
Expand Down Expand Up @@ -158,58 +160,35 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
}
private updateContextOfActiveNotebookKernel(activeEditor?: NotebookEditor) {
if (activeEditor && activeEditor.document.notebookType === JupyterNotebookView) {
this.notebookProvider
.getOrCreateNotebook({
identity: activeEditor.document.uri,
resource: activeEditor.document.uri,
getOnly: true
})
.then(async (nb) => {
if (activeEditor === this.vscNotebook.activeNotebookEditor) {
const canStart = nb && nb.status !== ServerStatus.NotStarted;
this.canRestartNotebookKernelContext.set(!!canStart).ignoreErrors();
const canInterrupt = nb && nb.status === ServerStatus.Busy;
this.canInterruptNotebookKernelContext.set(!!canInterrupt).ignoreErrors();
}
})
.catch(
traceError.bind(undefined, 'Failed to determine if a notebook is active for the current editor')
);
} else {
this.canRestartNotebookKernelContext.set(false).ignoreErrors();
this.canInterruptNotebookKernelContext.set(false).ignoreErrors();
const kernel = this.kernelProvider.get(activeEditor.document);
if (kernel) {
const canStart = kernel.status !== ServerStatus.NotStarted;
this.canRestartNotebookKernelContext.set(!!canStart).ignoreErrors();
const canInterrupt = kernel.status === ServerStatus.Busy;
const hasPendingCells = this.kernelProvider.get(activeEditor.document)?.hasPendingCells === true;
this.canInterruptNotebookKernelContext.set(canInterrupt || hasPendingCells).ignoreErrors();
return;
}
}
this.canRestartNotebookKernelContext.set(false).ignoreErrors();
this.canInterruptNotebookKernelContext.set(false).ignoreErrors();
}
private updateContextOfActiveInteractiveWindowKernel() {
const interactiveWindow = getActiveInteractiveWindow(this.interactiveProvider);
if (interactiveWindow?.notebookUri) {
this.notebookProvider
.getOrCreateNotebook({
identity: interactiveWindow.notebookUri,
resource: interactiveWindow.owner,
getOnly: true
})
.then(async (nb) => {
if (
getActiveInteractiveWindow(this.interactiveProvider) === interactiveWindow &&
nb !== undefined
) {
const canStart = nb && nb.status !== ServerStatus.NotStarted;
this.canRestartInteractiveWindowKernelContext.set(!!canStart).ignoreErrors();
const canInterrupt = nb && nb.status === ServerStatus.Busy;
this.canInterruptInteractiveWindowKernelContext.set(!!canInterrupt).ignoreErrors();
}
})
.catch(
traceError.bind(
undefined,
'Failed to determine if a kernel is active for the current interactive window'
)
);
} else {
this.canRestartInteractiveWindowKernelContext.set(false).ignoreErrors();
this.canInterruptInteractiveWindowKernelContext.set(false).ignoreErrors();
const notebook = interactiveWindow?.notebookEditor?.document;
if (notebook) {
const kernel = this.kernelProvider.get(notebook);
if (kernel) {
const canStart = kernel.status !== ServerStatus.NotStarted;
this.canRestartInteractiveWindowKernelContext.set(!!canStart).ignoreErrors();
const canInterrupt = kernel.status === ServerStatus.Busy;
const hasPendingCells = this.kernelProvider.get(notebook)?.hasPendingCells === true;
this.canInterruptInteractiveWindowKernelContext.set(canInterrupt || hasPendingCells).ignoreErrors();
return;
}
}
this.canRestartInteractiveWindowKernelContext.set(false).ignoreErrors();
this.canInterruptInteractiveWindowKernelContext.set(false).ignoreErrors();
}
private onDidKernelStatusChange({ notebook }: { status: ServerStatus; notebook: INotebook }) {
// Ok, kernel status has changed.
Expand Down Expand Up @@ -243,7 +222,7 @@ export class ActiveEditorContextService implements IExtensionSingleActivationSer
this.pythonOrInteractiveOrNativeContext
.set(
this.nativeContext.value === true ||
(this.interactiveContext.value === true && this.isPythonFileActive === true)
(this.interactiveContext.value === true && this.isPythonFileActive === true)
)
.ignoreErrors();
}
Expand Down
8 changes: 4 additions & 4 deletions src/client/datascience/jupyter/kernels/kernel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ export class Kernel implements IKernel {
private restarting?: Deferred<void>;
private readonly kernelExecution: KernelExecution;
private startCancellation = new CancellationTokenSource();
public get hasPendingCells() {
return this.kernelExecution.hasPendingCells;
}
constructor(
public readonly notebookDocument: NotebookDocument,
public readonly resourceUri: Resource,
Expand All @@ -126,7 +129,7 @@ export class Kernel implements IKernel {
private readonly configService: IConfigurationService,
outputTracker: CellOutputDisplayIdTracker,
private readonly workspaceService: IWorkspaceService,
private readonly cellHashProviderFactory: CellHashProviderFactory,
cellHashProviderFactory: CellHashProviderFactory,
private readonly pythonExecutionFactory: IPythonExecutionFactory,
notebookControllerManager: INotebookControllerManager
) {
Expand Down Expand Up @@ -156,9 +159,6 @@ export class Kernel implements IKernel {
sendKernelTelemetryEvent(this.resourceUri, Telemetry.ExecuteCell);
const stopWatch = new StopWatch();
const notebookPromise = this.startNotebook();
if (cell.notebook.notebookType === InteractiveWindowView) {
await this.cellHashProviderFactory.getOrCreate(this).addCellHash(cell);
}
const promise = this.kernelExecution.executeCell(notebookPromise, cell);
this.trackNotebookCellPerceivedColdTime(stopWatch, notebookPromise, promise).catch(noop);
await promise;
Expand Down
20 changes: 18 additions & 2 deletions src/client/datascience/jupyter/kernels/kernelExecution.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { CellExecutionQueue } from './cellExecutionQueue';
import type { IKernel, KernelConnectionMetadata } from './types';
import { NotebookCellRunState } from './types';
import { CellHashProviderFactory } from '../../editor-integration/cellHashProviderFactory';
import { InteractiveWindowView } from '../../notebook/constants';

/**
* Separate class that deals just with kernel execution.
Expand All @@ -32,6 +33,14 @@ export class KernelExecution implements IDisposable {
private readonly disposables: IDisposable[] = [];
private _interruptPromise?: Promise<InterruptResult>;
private _restartPromise?: Promise<void>;
private _hasPendingCells?: boolean;
public get hasPendingCells() {
if (this._hasPendingCells) {
return true;
}
const queue = this.documentExecutions.get(this.kernel.notebookDocument);
return queue ? !queue.isEmpty : false;
}
constructor(
private readonly kernel: IKernel,
errorHandler: IDataScienceErrorHandler,
Expand All @@ -41,7 +50,7 @@ export class KernelExecution implements IDisposable {
disposables: IDisposableRegistry,
controller: NotebookController,
outputTracker: CellOutputDisplayIdTracker,
cellHashProviderFactory: CellHashProviderFactory
private readonly cellHashProviderFactory: CellHashProviderFactory
) {
this.executionFactory = new CellExecutionFactory(
kernel,
Expand All @@ -53,19 +62,26 @@ export class KernelExecution implements IDisposable {
cellHashProviderFactory
);
}

public async executeCell(notebookPromise: Promise<INotebook>, cell: NotebookCell): Promise<NotebookCellRunState> {
if (cell.kind == NotebookCellKind.Markup) {
return NotebookCellRunState.Success;
}

// If we're in the middle of restarting, ensure we properly have the state of has pending cells.
this._hasPendingCells = true;

// If we're restarting, wait for it to finish
if (this._restartPromise) {
await this._restartPromise;
}
if (cell.notebook.notebookType === InteractiveWindowView) {
await this.cellHashProviderFactory.getOrCreate(this.kernel).addCellHash(cell);
}

const executionQueue = this.getOrCreateCellExecutionQueue(cell.notebook, notebookPromise);
executionQueue.queueCell(cell);
// Once we've queued the cell, then the queue will tell us whether we have any pending cells or not.
this._hasPendingCells = false;
const result = await executionQueue.waitForCompletion([cell]);
return result[0];
}
Expand Down
1 change: 1 addition & 0 deletions src/client/datascience/jupyter/kernels/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ export interface IKernel extends IAsyncDisposable {
readonly onWillRestart: Event<void>;
readonly onWillInterrupt: Event<void>;
readonly status: ServerStatus;
readonly hasPendingCells: boolean;
readonly disposed: boolean;
/**
* Kernel information, used to save in ipynb in the metadata.
Expand Down