Skip to content

Commit

Permalink
Main thread runtime notebook kernels (#5926)
Browse files Browse the repository at this point in the history
This PR adds notebook kernels for language runtimes in the main thread.
Before this, runtime kernels lived in the
`positron-notebook-controllers` extension. Having kernels in the main
thread gives us more control over runtime session management. Addresses
#2671.

Should also address #4224
because of the way we manage notebook runtime sessions.

Some changes were required in the runtime session service in order to
make this possible.

### Release Notes

#### New Features

- Positron's notebook kernels now live in the main thread rather than in
an extension. They should also be more stable. Since this was a big
change, the feature can be disabled via the
`positron.runtimeNotebookKernel.enable` setting.

#### Bug Fixes

- Cancelling the ipykernel installation modal no longer breaks kernel
selection (#4224).

### QA Notes

I'll run E2E tests against this branch. Playing around with notebooks
should also feel stable.

Should also address #4224.
  • Loading branch information
seeM authored Jan 10, 2025
1 parent 2ee0ebe commit 08c9ce4
Show file tree
Hide file tree
Showing 35 changed files with 3,116 additions and 235 deletions.
4 changes: 4 additions & 0 deletions build/lib/i18n.resources.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@
"name": "vs/workbench/services/runtimeStartup",
"project": "vscode-workbench"
},
{
"name": "vs/workbench/contrib/runtimeNotebookKernel",
"project": "vscode-workbench"
},
{
"name": "vs/workbench/services/voiceRecognition",
"project": "vscode-workbench"
Expand Down
17 changes: 0 additions & 17 deletions extensions/positron-notebook-controllers/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,6 @@
"enablement": "positron.hasRunningNotebookSession"
}
],
"configuration": [
{
"title": "Notebook Controllers",
"properties": {
"notebook.experimental.showExecutionInfo": {
"type": "boolean",
"default": false,
"description": "%notebook.experimental.showExecutionInfo.description%"
},
"python.languageServer": {
"type": "string",
"default": "Default",
"description": "%python.languageServer.description%"
}
}
}
],
"menus": {
"notebook/toolbar": [
{
Expand Down
4 changes: 1 addition & 3 deletions extensions/positron-notebook-controllers/package.nls.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
{
"positron.command.restartKernel.title": "Restart Kernel",
"positron.command.restartKernel.shortTitle": "Restart",
"python.languageServer.description": "This setting is included as a temporary workaround to disable the Jupyter extension's language server. The value should remain 'Default'.",
"notebook.experimental.showExecutionInfo.description": "Enable a status bar item that shows notebook execution info such as total duration and number of cells executing."
"positron.command.restartKernel.shortTitle": "Restart"
}
6 changes: 6 additions & 0 deletions extensions/positron-notebook-controllers/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ const _onDidSetHasRunningNotebookSessionContext = new vscode.EventEmitter<boolea
export const onDidSetHasRunningNotebookSessionContext = _onDidSetHasRunningNotebookSessionContext.event;

export async function activate(context: vscode.ExtensionContext): Promise<void> {
// If experimental runtime notebook kernels are enabled, exit early.
const enableRuntimeNotebookKernel = vscode.workspace.getConfiguration().get<boolean>('positron.runtimeNotebookKernel.enable');
if (enableRuntimeNotebookKernel) {
return;
}

context.subscriptions.push(_onDidSetHasRunningNotebookSessionContext);

const notebookSessionService = new NotebookSessionService();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import { SerializableObjectWithBuffers } from '../../../services/extensions/comm
import { isWebviewReplayMessage } from '../../../contrib/positronWebviewPreloads/browser/utils.js';
import { IPositronWebviewPreloadService } from '../../../services/positronWebviewPreloads/browser/positronWebviewPreloadService.js';
import { IPositronConnectionsService } from '../../../services/positronConnections/common/interfaces/positronConnectionsService.js';
import { IRuntimeNotebookKernelService } from '../../../contrib/runtimeNotebookKernel/browser/interfaces/runtimeNotebookKernelService.js';

/**
* Represents a language runtime event (for example a message or state change)
Expand Down Expand Up @@ -1137,6 +1138,7 @@ export class MainThreadLanguageRuntime
@ILanguageRuntimeService private readonly _languageRuntimeService: ILanguageRuntimeService,
@IRuntimeSessionService private readonly _runtimeSessionService: IRuntimeSessionService,
@IRuntimeStartupService private readonly _runtimeStartupService: IRuntimeStartupService,
@IRuntimeNotebookKernelService private readonly _runtimeNotebookKernelService: IRuntimeNotebookKernelService,
@IPositronConsoleService private readonly _positronConsoleService: IPositronConsoleService,
@IPositronDataExplorerService private readonly _positronDataExplorerService: IPositronDataExplorerService,
@IPositronVariablesService private readonly _positronVariablesService: IPositronVariablesService,
Expand All @@ -1154,6 +1156,7 @@ export class MainThreadLanguageRuntime
// TODO@softwarenerd - We needed to find a central place where we could ensure that certain
// Positron services were up and running early in the application lifecycle. For now, this
// is where we're doing this.
this._runtimeNotebookKernelService.initialize();
this._positronHelpService.initialize();
this._positronConsoleService.initialize();
this._positronDataExplorerService.initialize();
Expand Down
13 changes: 10 additions & 3 deletions src/vs/workbench/api/common/extHostConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,11 @@ export class ExtHostConfigProvider {
},
get: <T>(key: string, defaultValue?: T) => {
// --- Start Positron ---
// Disable vscode-jupyter's cell decorations, code lenses, and interactive window
// entrpoints, preferring Positron alternatives instead.
// TODO(seem): We can remove this if we eventually decide to unbundle vscode-jupyter.
// TODO(seem): We can remove this block if we eventually decide to unbundle vscode-jupyter.

if (section === 'jupyter') {
// Disable vscode-jupyter's cell decorations, code lenses, and interactive window
// entrpoints, preferring Positron alternatives instead.
switch (key) {
case 'interactiveWindow.cellMarker.decorateCells':
case 'interactiveWindow.codeLens.enable':
Expand All @@ -198,6 +199,12 @@ export class ExtHostConfigProvider {
case 'enableCellCodeLens':
return false;
}
} else if (section === 'python') {
switch (key) {
// Force to 'Default' to disable the Jupyter extension's language clients.
case 'languageServer':
return 'Default';
}
}
// --- End Positron ---
this._validateConfigurationAccess(section ? `${section}.${key}` : key, overrides, extensionDescription?.identifier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,26 @@ import { INotebookCellExecution, INotebookExecutionStateService } from '../../co
import { INotebookKernelHistoryService, INotebookKernelService } from '../../common/notebookKernelService.js';
import { INotebookLoggingService } from '../../common/notebookLoggingService.js';

// --- Start Positron ---
// Imports for the new start/end execution events.

// eslint-disable-next-line no-duplicate-imports
import { IDidStartNotebookCellsExecutionEvent, IDidEndNotebookCellsExecutionEvent } from '../../common/notebookExecutionService.js';
import { Emitter } from '../../../../../base/common/event.js';
// --- End Positron ---

export class NotebookExecutionService implements INotebookExecutionService, IDisposable {
declare _serviceBrand: undefined;
private _activeProxyKernelExecutionToken: CancellationTokenSource | undefined;
// --- Start Positron ---
// Add new start/end execution events.

private readonly _onDidStartNotebookCellsExecution = new Emitter<IDidStartNotebookCellsExecutionEvent>();
private readonly _onDidEndNotebookCellsExecution = new Emitter<IDidEndNotebookCellsExecutionEvent>();

public readonly onDidStartNotebookCellsExecution = this._onDidStartNotebookCellsExecution.event;
public readonly onDidEndNotebookCellsExecution = this._onDidEndNotebookCellsExecution.event;
// --- End Positron ---

constructor(
@ICommandService private readonly _commandService: ICommandService,
Expand Down Expand Up @@ -81,7 +97,18 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis
await this.runExecutionParticipants(validCellExecutions);

this._notebookKernelService.selectKernelForNotebook(kernel, notebook);
await kernel.executeNotebookCellsRequest(notebook.uri, validCellExecutions.map(c => c.cellHandle));
// --- Start Positron ---
// Wrap executeNotebookCellsRequest in a try-finally, and fire the new start/end execution events.
const startTime = Date.now();
const cellHandles = validCellExecutions.map(c => c.cellHandle);
this._onDidStartNotebookCellsExecution.fire({ cellHandles });
try {
await kernel.executeNotebookCellsRequest(notebook.uri, cellHandles);
} finally {
const duration = Date.now() - startTime;
this._onDidEndNotebookCellsExecution.fire({ cellHandles, duration });
}
// --- End Positron ---
// the connecting state can change before the kernel resolves executeNotebookCellsRequest
const unconfirmed = validCellExecutions.filter(exe => exe.state === NotebookCellExecutionState.Unconfirmed);
if (unconfirmed.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,10 @@ export class NotebookService extends Disposable implements INotebookService {
) {
super();

notebookRendererExtensionPoint.setHandler((renderers) => {
// --- Start Positron ---
// Register leaked disposable.
this._register(notebookRendererExtensionPoint.setHandler((renderers) => {
// --- End Positron ---
this._notebookRenderersInfoStore.clear();

for (const extension of renderers) {
Expand Down Expand Up @@ -577,9 +580,15 @@ export class NotebookService extends Disposable implements INotebookService {
}

this._onDidChangeOutputRenderers.fire();
});
// --- Start Positron ---
// Register leaked disposable.
}));
// --- End Positron ---

notebookPreloadExtensionPoint.setHandler(extensions => {
// --- Start Positron ---
// Register leaked disposable.
this._register(notebookPreloadExtensionPoint.setHandler(extensions => {
// --- End Positron ---
this._notebookStaticPreloadInfoStore.clear();

for (const extension of extensions) {
Expand Down Expand Up @@ -623,7 +632,10 @@ export class NotebookService extends Disposable implements INotebookService {
}));
}
}
});
// --- Start Positron ---
// Register leaked disposable.
}));
// --- End Positron ---

const updateOrder = () => {
this._displayOrder = new MimeTypeDisplayOrder(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { EnablementState } from '../../../../services/extensionManagement/common
// --- Start Positron ---
// eslint-disable-next-line no-duplicate-imports
import { DeferredPromise } from '../../../../../base/common/async.js';
import { POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID } from '../../../runtimeNotebookKernel/common/runtimeNotebookKernelConfig.js';
// --- End Positron ---
type KernelPick = IQuickPickItem & { kernel: INotebookKernel };
function isKernelPick(item: QuickPickInput<IQuickPickItem>): item is KernelPick {
Expand Down Expand Up @@ -586,7 +587,8 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
// --- Start Positron ---
// Enable kernel source action providers for Positron kernels
const kernel = all.find(kernel => kernel.id === `ms-toolsai.jupyter/${selectedKernelId}` ||
kernel.id === `positron.positron-notebook-controllers/${selectedKernelId}`);
kernel.id === `positron.positron-notebook-controllers/${selectedKernelId}` ||
kernel.id === `${POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID}/${selectedKernelId}`);
// --- End Positron ---
if (kernel) {
await this._selecteKernel(notebook, kernel);
Expand Down Expand Up @@ -648,9 +650,10 @@ export class KernelPickerMRUStrategy extends KernelPickerStrategyBase {
}

// --- Start Positron ---
// Remove the "Positron Notebook Controllers" quickpick item in favor of kernel source action providers
// Remove Positron quickpick items in favor of kernel source action providers
const others = matchResult.all.filter(item => item.extension.value !== JUPYTER_EXTENSION_ID &&
item.extension.value !== 'positron.positron-notebook-controllers');
item.extension.value !== 'positron.positron-notebook-controllers' &&
item.extension.value !== POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID);
// --- End Positron ---
const quickPickItems: QuickPickInput<KernelQuickPickItem>[] = [];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,26 @@ import { createDecorator } from '../../../../platform/instantiation/common/insta
import { NotebookCellTextModel } from './model/notebookCellTextModel.js';
import { INotebookTextModel, IOutputDto, IOutputItemDto } from './notebookCommon.js';
import { INotebookCellExecution } from './notebookExecutionStateService.js';
// --- Start Positron ---
// Add start/end execution events.

import { Event } from '../../../../base/common/event.js';

/** An event that fires when a notebook cells execution is started. */
export interface IDidStartNotebookCellsExecutionEvent {
/** The handles of the cells being executed. */
cellHandles: number[];
}

/** An event that fires when a notebook cells execution is ended. */
export interface IDidEndNotebookCellsExecutionEvent {
/** The handles of the cells that were executed. */
cellHandles: number[];

/** The duration of the execution in milliseconds. */
duration: number;
}
// --- End Positron ---

export enum CellExecutionUpdateType {
Output = 1,
Expand All @@ -34,6 +54,15 @@ export const INotebookExecutionService = createDecorator<INotebookExecutionServi

export interface INotebookExecutionService {
_serviceBrand: undefined;
// --- Start Positron ---
// Add start/end execution events.

/** An event that fires when a notebook cells execution is started. */
onDidStartNotebookCellsExecution: Event<IDidStartNotebookCellsExecutionEvent>;

/** An event that fires when a notebook cells execution is ended. */
onDidEndNotebookCellsExecution: Event<IDidEndNotebookCellsExecutionEvent>;
// --- End Positron ---

executeNotebookCells(notebook: INotebookTextModel, cells: Iterable<NotebookCellTextModel>, contextKeyService: IContextKeyService): Promise<void>;
cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable<NotebookCellTextModel>): Promise<void>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,26 @@ import { RuntimeClientType } from '../../../../services/runtimeSession/common/ru
import { TestLanguageRuntimeSession } from '../../../../services/runtimeSession/test/common/testLanguageRuntimeSession.js';
import { startTestLanguageRuntimeSession } from '../../../../services/runtimeSession/test/common/testRuntimeSessionService.js';
import { PositronTestServiceAccessor, positronWorkbenchInstantiationService } from '../../../../test/browser/positronWorkbenchTestServices.js';
import { TestNotebookService } from '../../../../test/common/positronWorkbenchTestServices.js';
import { INotebookRendererInfo, INotebookStaticPreloadInfo } from '../../../notebook/common/notebookCommon.js';
import { NotebookOutputRendererInfo } from '../../../notebook/common/notebookOutputRenderer.js';
import { ExtensionIdentifier } from '../../../../../platform/extensions/common/extensions.js';

class TestNotebookService implements Partial<INotebookService> {
getRenderers(): INotebookRendererInfo[] {
return [];
}

getPreferredRenderer(_mimeType: string): NotebookOutputRendererInfo | undefined {
return <NotebookOutputRendererInfo>{
id: 'positron-ipywidgets',
extensionId: new ExtensionIdentifier('positron.positron-ipywidgets'),
};
}

*getStaticPreloads(_viewType: string): Iterable<INotebookStaticPreloadInfo> {
// Yield nothing.
}
}

interface TestNotebookEditor extends INotebookEditor {
changeModel(uri: URI): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { selectKernelIcon } from '../../notebook/browser/notebookIcons.js';
import { INotebookKernelService, INotebookKernel } from '../../notebook/common/notebookKernelService.js';
import { PositronNotebookInstance } from './PositronNotebookInstance.js';
import { IPositronNotebookService } from '../../../services/positronNotebook/browser/positronNotebookService.js';
import { POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID } from '../../runtimeNotebookKernel/common/runtimeNotebookKernelConfig.js';

export const SELECT_KERNEL_ID_POSITRON = 'positronNotebook.selectKernel';
const NOTEBOOK_ACTIONS_CATEGORY_POSITRON = localize2('positronNotebookActions.category', 'Positron Notebook');
Expand Down Expand Up @@ -63,7 +64,9 @@ class SelectPositronNotebookKernelAction extends Action2 {

const gatherKernelPicks = () => {
const kernelMatches = notebookKernelService.getMatchingKernel(notebook);
const positronKernels = kernelMatches.all.filter(k => k.extension.value === 'positron.positron-notebook-controllers');
const positronKernels = kernelMatches.all.filter(k =>
k.extension.value === 'positron.positron-notebook-controllers' ||
k.extension.value === POSITRON_RUNTIME_NOTEBOOK_KERNELS_EXTENSION_ID);
if (positronKernels.length === 0) {
quickPick.busy = true;
quickPick.items = [{ label: localize('positronNotebookActions.selectKernel.noKernel', 'No Positron Notebook Kernel found'), picked: true }];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2025 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

import { createDecorator } from '../../../../../platform/instantiation/common/instantiation.js';

// Create the decorator for the service (used in dependency injection).
export const IRuntimeNotebookKernelService = createDecorator<IRuntimeNotebookKernelService>('runtimeNotebookKernelService');

export interface IRuntimeNotebookKernelService {
/**
* Needed for service branding in dependency injector.
*/
readonly _serviceBrand: undefined;

/**
* Placeholder that gets called to "initialize" the service.
*/
initialize(): void;
}
Loading

0 comments on commit 08c9ce4

Please sign in to comment.