Skip to content

Commit

Permalink
Draft for gather extensibility point (microsoft#13672)
Browse files Browse the repository at this point in the history
* draft for gather extensibility point

* fix compiler errors

* add event on extension api

* added an event to the api

* oops

* remove old implementation

* fix activation errors

* use 3 events instead of 1

* create a class for the execution logger events
- part 1

* put the logger into a class

* rename logger to notebook extensibility
and made it a singleton

* oops

* fix functional tests

* fix more functional tests

* remove onNotebookOpened, especify event name as
post execute

* moved to the datascience part of the API

* add news
  • Loading branch information
David Kutugata committed Sep 11, 2020
1 parent ed3f29f commit c9f2d9c
Show file tree
Hide file tree
Showing 14 changed files with 94 additions and 14 deletions.
1 change: 1 addition & 0 deletions news/1 Enhancements/13306.md
@@ -0,0 +1 @@
Update API to expose events for cell excecution and kernel restart.
10 changes: 8 additions & 2 deletions src/client/api.ts
Expand Up @@ -4,11 +4,12 @@
'use strict';

import { Event, Uri } from 'vscode';
import { NotebookCell } from 'vscode-proposed';
import { isTestExecution } from './common/constants';
import { traceError } from './common/logger';
import { IConfigurationService, Resource } from './common/types';
import { IDataViewerDataProvider, IDataViewerFactory } from './datascience/data-viewing/types';
import { IJupyterUriProvider, IJupyterUriProviderRegistration } from './datascience/types';
import { IJupyterUriProvider, IJupyterUriProviderRegistration, INotebookExtensibility } from './datascience/types';
import { getDebugpyLauncherArgs, getDebugpyPackagePath } from './debugger/extension/adapter/remoteLaunchers';
import { IInterpreterService } from './interpreter/contracts';
import { IServiceContainer, IServiceManager } from './ioc/types';
Expand Down Expand Up @@ -77,6 +78,8 @@ export interface IExtensionApi {
};
};
datascience: {
readonly onKernelPostExecute: Event<NotebookCell>;
readonly onKernelRestart: Event<void>;
/**
* Launches Data Viewer component.
* @param {IDataViewerDataProvider} dataProvider Instance that will be used by the Data Viewer component to fetch data.
Expand All @@ -99,6 +102,7 @@ export function buildApi(
): IExtensionApi {
const configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
const interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
const notebookExtensibility = serviceContainer.get<INotebookExtensibility>(INotebookExtensibility);
const api: IExtensionApi = {
// 'ready' will propagate the exception, but we must log it here first.
ready: ready.catch((ex) => {
Expand Down Expand Up @@ -139,7 +143,9 @@ export function buildApi(
IJupyterUriProviderRegistration
);
container.registerProvider(picker);
}
},
onKernelPostExecute: notebookExtensibility.onKernelPostExecute,
onKernelRestart: notebookExtensibility.onKernelRestart
}
};

Expand Down
7 changes: 6 additions & 1 deletion src/client/datascience/interactive-ipynb/nativeEditor.ts
Expand Up @@ -65,6 +65,7 @@ import {
INotebookEditor,
INotebookEditorProvider,
INotebookExporter,
INotebookExtensibility,
INotebookImporter,
INotebookMetadataLive,
INotebookModel,
Expand Down Expand Up @@ -93,6 +94,9 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
public get onDidChangeViewState(): Event<void> {
return this._onDidChangeViewState.event;
}
public get notebookExtensibility(): INotebookExtensibility {
return this.nbExtensibility;
}

public get visible(): boolean {
return this.viewState.visible;
Expand Down Expand Up @@ -181,7 +185,8 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
expService: IExperimentService,
private _model: INotebookModel,
webviewPanel: WebviewPanel | undefined,
selector: KernelSelector
selector: KernelSelector,
private nbExtensibility: INotebookExtensibility
) {
super(
listeners,
Expand Down
Expand Up @@ -41,6 +41,7 @@ import {
IJupyterVariables,
INotebookEditorProvider,
INotebookExporter,
INotebookExtensibility,
INotebookImporter,
INotebookModel,
INotebookProvider,
Expand Down Expand Up @@ -103,7 +104,8 @@ export class NativeEditorOldWebView extends NativeEditor {
expService: IExperimentService,
model: INotebookModel,
webviewPanel: WebviewPanel | undefined,
selector: KernelSelector
selector: KernelSelector,
notebookExtensibility: INotebookExtensibility
) {
super(
listeners,
Expand Down Expand Up @@ -138,7 +140,8 @@ export class NativeEditorOldWebView extends NativeEditor {
expService,
model,
webviewPanel,
selector
selector,
notebookExtensibility
);
asyncRegistry.push(this);
// No ui syncing in old notebooks.
Expand Down
Expand Up @@ -48,6 +48,7 @@ import {
INotebookEditor,
INotebookEditorProvider,
INotebookExporter,
INotebookExtensibility,
INotebookImporter,
INotebookModel,
INotebookProvider,
Expand Down Expand Up @@ -228,7 +229,8 @@ export class NativeEditorProviderOld extends NativeEditorProvider {
this.serviceContainer.get<IExperimentService>(IExperimentService),
model,
panel,
this.serviceContainer.get<KernelSelector>(KernelSelector)
this.serviceContainer.get<KernelSelector>(KernelSelector),
this.serviceContainer.get<INotebookExtensibility>(INotebookExtensibility)
);
this.activeEditors.set(model.file.fsPath, editor);
this.disposables.push(editor.closed(this.onClosedEditor.bind(this)));
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/jupyter/kernels/cellExecution.ts
Expand Up @@ -194,7 +194,7 @@ export class CellExecution {
if (editor && !(editor instanceof NotebookEditor)) {
throw new Error('Executing Notebook with another Editor');
}
editor.notifyExecution(this.cell.document.getText());
editor.notifyExecution(this.cell);
}

/**
Expand Down
15 changes: 11 additions & 4 deletions src/client/datascience/notebook/notebookEditor.ts
Expand Up @@ -4,7 +4,7 @@
'use strict';

import { ConfigurationTarget, Event, EventEmitter, Uri, WebviewPanel } from 'vscode';
import type { NotebookDocument } from 'vscode-proposed';
import type { NotebookCell, NotebookDocument } from 'vscode-proposed';
import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../common/application/types';
import { traceError } from '../../common/logger';
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
Expand All @@ -17,6 +17,7 @@ import { IKernel, IKernelProvider } from '../jupyter/kernels/types';
import {
INotebook,
INotebookEditor,
INotebookExtensibility,
INotebookModel,
INotebookProvider,
InterruptResult,
Expand Down Expand Up @@ -62,6 +63,9 @@ export class NotebookEditor implements INotebookEditor {
public get onExecutedCode(): Event<string> {
return this.executedCode.event;
}
public get notebookExtensibility(): INotebookExtensibility {
return this.nbExtensibility;
}
public notebook?: INotebook | undefined;

private changedViewState = new EventEmitter<void>();
Expand All @@ -81,7 +85,8 @@ export class NotebookEditor implements INotebookEditor {
private readonly statusProvider: IStatusProvider,
private readonly applicationShell: IApplicationShell,
private readonly configurationService: IConfigurationService,
disposables: IDisposableRegistry
disposables: IDisposableRegistry,
private readonly nbExtensibility: INotebookExtensibility
) {
disposables.push(model.onDidEdit(() => this._modified.fire(this)));
disposables.push(
Expand Down Expand Up @@ -161,9 +166,10 @@ export class NotebookEditor implements INotebookEditor {
cell.metadata.outputCollapsed = true;
});
}
public notifyExecution(code: string) {
public notifyExecution(cell: NotebookCell) {
this._executed.fire(this);
this.executedCode.fire(code);
this.executedCode.fire(cell.document.getText());
this.nbExtensibility.fireKernelPostExecute(cell);
}
public async interruptKernel(): Promise<void> {
if (this.restartingKernel) {
Expand Down Expand Up @@ -235,6 +241,7 @@ export class NotebookEditor implements INotebookEditor {

try {
await kernel.restart();
this.nbExtensibility.fireKernelRestart();
} catch (exc) {
// If we get a kernel promise failure, then restarting timed out. Just shutdown and restart the entire server.
// Note, this code might not be necessary, as such an error is thrown only when interrupting a kernel times out.
Expand Down
7 changes: 5 additions & 2 deletions src/client/datascience/notebook/notebookEditorProvider.ts
Expand Up @@ -21,6 +21,7 @@ import {
IDataScienceFileSystem,
INotebookEditor,
INotebookEditorProvider,
INotebookExtensibility,
INotebookProvider,
IStatusProvider
} from '../types';
Expand Down Expand Up @@ -68,7 +69,8 @@ export class NotebookEditorProvider implements INotebookEditorProvider {
@inject(IApplicationShell) private readonly appShell: IApplicationShell,
@inject(IStatusProvider) private readonly statusProvider: IStatusProvider,
@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer,
@inject(IDataScienceFileSystem) private readonly fs: IDataScienceFileSystem
@inject(IDataScienceFileSystem) private readonly fs: IDataScienceFileSystem,
@inject(INotebookExtensibility) private readonly notebookExtensibility: INotebookExtensibility
) {
this.disposables.push(this.vscodeNotebook.onDidOpenNotebookDocument(this.onDidOpenNotebookDocument, this));
this.disposables.push(this.vscodeNotebook.onDidCloseNotebookDocument(this.onDidCloseNotebookDocument, this));
Expand Down Expand Up @@ -162,7 +164,8 @@ export class NotebookEditorProvider implements INotebookEditorProvider {
this.statusProvider,
this.appShell,
this.configurationService,
this.disposables
this.disposables,
this.notebookExtensibility
);
this.onEditorOpened(editor);
}
Expand Down
27 changes: 27 additions & 0 deletions src/client/datascience/notebookExtensibility.ts
@@ -0,0 +1,27 @@
import { injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import type { NotebookCell } from 'vscode-proposed';
import { INotebookExtensibility } from './types';

@injectable()
export class NotebookExtensibility implements INotebookExtensibility {
private kernelExecute = new EventEmitter<NotebookCell>();

private kernelRestart = new EventEmitter<void>();

public get onKernelPostExecute(): Event<NotebookCell> {
return this.kernelExecute.event;
}

public get onKernelRestart(): Event<void> {
return this.kernelRestart.event;
}

public fireKernelRestart(): void {
this.kernelRestart.fire();
}

public fireKernelPostExecute(cell: NotebookCell): void {
this.kernelExecute.fire(cell);
}
}
Expand Up @@ -55,6 +55,7 @@ import {
INotebookEditor,
INotebookEditorProvider,
INotebookExporter,
INotebookExtensibility,
INotebookImporter,
INotebookModel,
INotebookProvider,
Expand Down Expand Up @@ -264,7 +265,8 @@ export class NativeEditorProvider implements INotebookEditorProvider, CustomEdit
this.serviceContainer.get<IExperimentService>(IExperimentService),
model,
panel,
this.serviceContainer.get<KernelSelector>(KernelSelector)
this.serviceContainer.get<KernelSelector>(KernelSelector),
this.serviceContainer.get<INotebookExtensibility>(INotebookExtensibility)
);
this.openedEditor(editor);
return editor;
Expand Down
3 changes: 3 additions & 0 deletions src/client/datascience/serviceRegistry.ts
Expand Up @@ -118,6 +118,7 @@ import { NotebookEditorProvider } from './notebook/notebookEditorProvider';
import { NotebookEditorProviderWrapper } from './notebook/notebookEditorProviderWrapper';
import { registerTypes as registerNotebookTypes } from './notebook/serviceRegistry';
import { NotebookAndInteractiveWindowUsageTracker } from './notebookAndInteractiveTracker';
import { NotebookExtensibility } from './notebookExtensibility';
import { NotebookModelFactory } from './notebookStorage/factory';
import { NativeEditorProvider } from './notebookStorage/nativeEditorProvider';
import { NativeEditorStorage } from './notebookStorage/nativeEditorStorage';
Expand Down Expand Up @@ -168,6 +169,7 @@ import {
INotebookEditorProvider,
INotebookExecutionLogger,
INotebookExporter,
INotebookExtensibility,
INotebookImporter,
INotebookProvider,
INotebookServer,
Expand Down Expand Up @@ -315,6 +317,7 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<ITrustService>(ITrustService, TrustService);
serviceManager.addSingleton<IDataScienceFileSystem>(IDataScienceFileSystem, DataScienceFileSystem);
serviceManager.addSingleton<IFileSystemPathUtils>(IFileSystemPathUtils, FileSystemPathUtils);
serviceManager.addSingleton<INotebookExtensibility>(INotebookExtensibility, NotebookExtensibility);

registerGatherTypes(serviceManager);
registerNotebookTypes(serviceManager);
Expand Down
11 changes: 11 additions & 0 deletions src/client/datascience/types.ts
Expand Up @@ -23,6 +23,7 @@ import {
Uri
} from 'vscode';
import { DebugProtocol } from 'vscode-debugprotocol';
import type { NotebookCell } from 'vscode-proposed';
import type { Data as WebSocketData } from 'ws';
import { ServerStatus } from '../../datascience-ui/interactive-common/mainState';
import { ICommandManager, IDebugService } from '../common/application/types';
Expand Down Expand Up @@ -566,6 +567,7 @@ export interface INotebookEditor extends Disposable {
readonly executed: Event<INotebookEditor>;
readonly modified: Event<INotebookEditor>;
readonly saved: Event<INotebookEditor>;
readonly notebookExtensibility: INotebookExtensibility;
/**
* Is this notebook representing an untitled file which has never been saved yet.
*/
Expand Down Expand Up @@ -593,6 +595,15 @@ export interface INotebookEditor extends Disposable {
restartKernel(): Promise<void>;
}

export const INotebookExtensibility = Symbol('INotebookExtensibility');

export interface INotebookExtensibility {
readonly onKernelPostExecute: Event<NotebookCell>;
readonly onKernelRestart: Event<void>;
fireKernelRestart(): void;
fireKernelPostExecute(cell: NotebookCell): void;
}

export const IInteractiveWindowListener = Symbol('IInteractiveWindowListener');

/**
Expand Down
7 changes: 7 additions & 0 deletions src/test/api.functional.test.ts
Expand Up @@ -14,6 +14,8 @@ import { buildApi } from '../client/api';
import { ConfigurationService } from '../client/common/configuration/service';
import { EXTENSION_ROOT_DIR } from '../client/common/constants';
import { IConfigurationService } from '../client/common/types';
import { NotebookExtensibility } from '../client/datascience/notebookExtensibility';
import { INotebookExtensibility } from '../client/datascience/types';
import { IInterpreterService } from '../client/interpreter/contracts';
import { InterpreterService } from '../client/interpreter/interpreterService';
import { ServiceContainer } from '../client/ioc/container';
Expand All @@ -29,17 +31,22 @@ suite('Extension API', () => {
let serviceManager: IServiceManager;
let configurationService: IConfigurationService;
let interpreterService: IInterpreterService;
let notebookExtensibility: INotebookExtensibility;

setup(() => {
serviceContainer = mock(ServiceContainer);
serviceManager = mock(ServiceManager);
configurationService = mock(ConfigurationService);
interpreterService = mock(InterpreterService);
notebookExtensibility = mock(NotebookExtensibility);

when(serviceContainer.get<IConfigurationService>(IConfigurationService)).thenReturn(
instance(configurationService)
);
when(serviceContainer.get<IInterpreterService>(IInterpreterService)).thenReturn(instance(interpreterService));
when(serviceContainer.get<INotebookExtensibility>(INotebookExtensibility)).thenReturn(
instance(notebookExtensibility)
);
});

test('Execution details settings API returns expected object if interpreter is set', async () => {
Expand Down
3 changes: 3 additions & 0 deletions src/test/datascience/dataScienceIocContainer.ts
Expand Up @@ -269,6 +269,7 @@ import { KernelFinder } from '../../client/datascience/kernel-launcher/kernelFin
import { KernelLauncher } from '../../client/datascience/kernel-launcher/kernelLauncher';
import { IKernelFinder, IKernelLauncher } from '../../client/datascience/kernel-launcher/types';
import { NotebookAndInteractiveWindowUsageTracker } from '../../client/datascience/notebookAndInteractiveTracker';
import { NotebookExtensibility } from '../../client/datascience/notebookExtensibility';
import { NotebookModelFactory } from '../../client/datascience/notebookStorage/factory';
import { NativeEditorStorage } from '../../client/datascience/notebookStorage/nativeEditorStorage';
import {
Expand Down Expand Up @@ -320,6 +321,7 @@ import {
INotebookEditorProvider,
INotebookExecutionLogger,
INotebookExporter,
INotebookExtensibility,
INotebookImporter,
INotebookProvider,
INotebookServer,
Expand Down Expand Up @@ -589,6 +591,7 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
IInterpreterHashProviderFactory,
InterpeterHashProviderFactory
);
this.serviceManager.addSingleton<INotebookExtensibility>(INotebookExtensibility, NotebookExtensibility);
this.serviceManager.addSingleton<IExportManager>(IExportManager, ExportManager);
this.serviceManager.addSingleton<ExportDependencyChecker>(ExportDependencyChecker, ExportDependencyChecker);
this.serviceManager.addSingleton<ExportFileOpener>(ExportFileOpener, ExportFileOpener);
Expand Down

0 comments on commit c9f2d9c

Please sign in to comment.