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

Updates to the Juptyer Server API #14233

Merged
merged 2 commits into from
Sep 1, 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
26 changes: 26 additions & 0 deletions src/api.deprecated.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

// eslint-disable-next-line @typescript-eslint/no-unused-vars
import { Uri } from 'vscode';

declare module './api' {
export interface JupyterServerCommandProvider {
/**
* Returns a list of commands to be displayed to the user.
* @deprecated Use `provideCommands` instead.
*/
commands?: JupyterServerCommand[];
}

export interface JupyterServerCommand {
/**
* @deprecated Use `label` instead.
*/
title?: string;
/**
* @deprecated Use `description` instead.
*/
detail?: string;
}
}
9 changes: 1 addition & 8 deletions src/api.internal.d.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 { CancellationToken, Event, QuickPickItem, Uri } from 'vscode';
import { Event, QuickPickItem, Uri } from 'vscode';

// These types are only used internally within the extension.
// Never to be exposed to other extensions.
Expand All @@ -27,13 +27,6 @@ declare module './api' {
*/
removeJupyterServer?(server: JupyterServer): Promise<void>;
}
export interface JupyterServerCommandProvider {
/**
* Returns a list of commands to be displayed to the user.
* @param value The value entered by the user in the quick pick.
*/
provideCommands(value: string, token: CancellationToken): Promise<JupyterServerCommand[]>;
}

export interface IJupyterUriProvider {
/**
Expand Down
35 changes: 14 additions & 21 deletions src/api.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// To use these types please reach out to the Jupyter Extension team (file an issue on the Jupyter Extension GitHub repo).
// Or please wait for these to be finalized and released.

import { CancellationToken } from 'vscode';
import { CancellationToken, ProviderResult } from 'vscode';
import { Event, Uri } from 'vscode';

declare module './api' {
Expand Down Expand Up @@ -97,28 +97,25 @@ declare module './api' {
/**
* Returns the list of servers.
*/
provideJupyterServers(token: CancellationToken): Promise<JupyterServer[]>;
provideJupyterServers(token: CancellationToken): ProviderResult<JupyterServer[]>;
/**
* Returns the connection information for the Jupyter server.
* It is OK to return the given `server`. When no result is returned, the given `server` will be used.
*/
resolveJupyterServer(server: JupyterServer, token: CancellationToken): Promise<ResolvedJupyterServer>;
resolveJupyterServer(server: JupyterServer, token: CancellationToken): ProviderResult<ResolvedJupyterServer>;
}
/**
* Represents a reference to a Jupyter Server command.
*/
export interface JupyterServerCommand {
/**
* Title of the command, like `save`.
* A human-readable string which is rendered prominent.
*/
title: string;
/**
* A human-readable string which is rendered less prominent in a separate line.
*/
detail?: string;
label: string;
/**
* A tooltip for the command, when represented in the UI.
* A human-readable string which is rendered less prominent on the same line.
*/
tooltip?: string;
description?: string;
}
/**
* Provider of Jupyter Server Commands.
Expand All @@ -127,20 +124,16 @@ declare module './api' {
export interface JupyterServerCommandProvider {
/**
* Returns a list of commands to be displayed to the user.
* @param value The value entered by the user in the quick pick.
*/
commands: JupyterServerCommand[];
provideCommands(value: string | undefined, token: CancellationToken): Promise<JupyterServerCommand[]>;
/**
* Invoked when a command has been selected.
* @param command The command selected by the user.
* @returns
* - JupyterServer : The Jupyter Server object that was created
* - 'back' : Go back to the previous UI in the workflow
* - undefined|void : Do nothing
*/
handleCommand(
command: JupyterServerCommand,
token: CancellationToken
): Promise<JupyterServer | 'back' | undefined | void>;
* @returns The Jupyter Server or a thenable that resolves to such. The lack of a result can be
* signaled by returning `undefined` or `null`.
*/
handleCommand(command: JupyterServerCommand, token: CancellationToken): ProviderResult<JupyterServer>;
}
export interface JupyterServerCollection {
/**
Expand Down
6 changes: 4 additions & 2 deletions src/kernels/jupyter/clearJupyterServersCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ export class ClearJupyterServersCommand implements IExtensionSyncActivationServi
return;
}
const token = new CancellationTokenSource();
const servers = await provider.serverProvider.provideJupyterServers(token.token);
const servers = await Promise.resolve(
provider.serverProvider.provideJupyterServers(token.token)
);
await Promise.all(
servers.map((server) =>
(servers || []).map((server) =>
provider.serverProvider!.removeJupyterServer!(server).catch(noop)
)
);
Expand Down
84 changes: 48 additions & 36 deletions src/kernels/jupyter/connection/jupyterServerProviderRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { CancellationToken, CancellationTokenSource, EventEmitter, QuickPickItem, Uri } from 'vscode';
import {
CancellationError,
CancellationToken,
CancellationTokenSource,
EventEmitter,
QuickPickItem,
Uri
} from 'vscode';
import {
IJupyterServerUri,
IJupyterUriProvider,
Expand All @@ -17,7 +24,7 @@ import { IDisposable, IDisposableRegistry } from '../../../platform/common/types
import { inject, injectable } from 'inversify';
import { disposeAllDisposables, stripCodicons } from '../../../platform/common/helpers';
import { traceError } from '../../../platform/logging';
import { JUPYTER_HUB_EXTENSION_ID, JVSC_EXTENSION_ID } from '../../../platform/common/constants';
import { JVSC_EXTENSION_ID } from '../../../platform/common/constants';

export class JupyterServerCollectionImpl extends Disposables implements JupyterServerCollection {
private _serverProvider?: JupyterServerProvider;
Expand Down Expand Up @@ -99,32 +106,33 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
}
async getQuickPickEntryItems(
value?: string
): Promise<(QuickPickItem & { default?: boolean | undefined; command?: JupyterServerCommand })[]> {
): Promise<(QuickPickItem & { default?: boolean | undefined; command: JupyterServerCommand })[]> {
if (!this.provider.commandProvider) {
return [];
}
const token = new CancellationTokenSource();
const isProposedApiAllowed =
this.provider.extensionId === JVSC_EXTENSION_ID || this.provider.extensionId === JUPYTER_HUB_EXTENSION_ID;
try {
value = isProposedApiAllowed ? value : undefined;
let items: JupyterServerCommand[] = [];
if (isProposedApiAllowed) {
if (this.provider.commandProvider.provideCommands) {
items = await this.provider.commandProvider.provideCommands(value || '', token.token);
if (
(!items || !items.length) &&
Array.isArray(this.provider.commandProvider.commands) &&
this.provider.commandProvider.commands.length
) {
items = this.provider.commandProvider.commands;
}
} else {
items = this.provider.commandProvider.commands;
items = this.provider.commandProvider.commands || [];
}
if (isProposedApiAllowed) {
if (!value) {
this.commands.clear();
}
items.forEach((c) => this.commands.set(c.title, c));
if (!value) {
this.commands.clear();
}
items.forEach((c) => this.commands.set(c.label || c.title || '', c));
return items.map((c) => {
return {
label: stripCodicons(c.title),
detail: stripCodicons(c.detail),
tooltip: c.tooltip,
label: stripCodicons(c.label || c.title),
detail: stripCodicons(c.description || c.detail),
command: c
};
});
Expand All @@ -139,33 +147,30 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
}
}
async handleQuickPick(
item: QuickPickItem & { command?: JupyterServerCommand },
item: QuickPickItem & { command: JupyterServerCommand },
_backEnabled: boolean
): Promise<string | undefined> {
if (!this.provider.commandProvider) {
throw new Error(`No Jupyter Server Command Provider for ${this.provider.extensionId}#${this.provider.id}`);
}
const token = new CancellationTokenSource();
try {
let command: JupyterServerCommand | undefined =
'command' in item ? (item.command as JupyterServerCommand) : undefined;
if (!command) {
command =
this.provider.commandProvider.commands.find((c) => c.title === item.label) ||
this.commands.get(item.label);
}
const command = item.command;
if (!command) {
throw new Error(
`Jupyter Server Command ${item.label} not found in Command Provider ${this.provider.extensionId}#${this.provider.id}`
);
}
try {
const result = await this.provider.commandProvider.handleCommand(command, token.token);
if (result === 'back') {
return result;
const result = await Promise.resolve(this.provider.commandProvider.handleCommand(command, token.token));
if (!result) {
return 'back';
}
return result?.id;
return result.id;
} catch (ex) {
if (ex instanceof CancellationError) {
return;
}
traceError(
`Failed to execute Jupyter Server Command ${item.label} in Command Provider ${this.provider.extensionId}#${this.provider.id}`,
ex
Expand Down Expand Up @@ -196,10 +201,15 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
};
}
if (this.provider.serverProvider?.resolveJupyterServer) {
const { connectionInformation: info } = await this.provider.serverProvider?.resolveJupyterServer(
server,
token.token
const result = await Promise.resolve(
this.provider.serverProvider?.resolveJupyterServer(server, token.token)
);
const info = result?.connectionInformation || server.connectionInformation;
if (!info?.baseUrl) {
throw new Error(
`Jupyter Provider ${this.id} does not implement the method resolveJupyterServer and/or baseUrl not returned`
);
}
return {
baseUrl: info.baseUrl.toString(),
displayName: server.label,
Expand All @@ -218,8 +228,8 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
if (this.provider.serverProvider) {
const token = new CancellationTokenSource();
try {
const servers = await this.getServers(token.token);
return servers.map((s) => s.id);
const servers = await Promise.resolve(this.getServers(token.token));
return (servers || []).map((s) => s.id);
} catch (ex) {
traceError(`Failed to get Jupyter Servers from ${this.provider.extensionId}#${this.provider.id}`, ex);
return [];
Expand All @@ -246,7 +256,9 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
async getServer(handle: string, token: CancellationToken): Promise<JupyterServer> {
const server =
this._servers.get(handle) ||
(await this.getServers(token).then((servers) => servers.find((s) => s.id === handle)));
(await Promise.resolve(this.getServers(token)).then((servers) =>
(servers || []).find((s) => s.id === handle)
));
if (server) {
return server;
}
Expand All @@ -262,9 +274,9 @@ class JupyterUriProviderAdaptor extends Disposables implements IJupyterUriProvid
if (!this.provider.serverProvider) {
throw new Error(`No Jupyter Server Provider for ${this.provider.extensionId}#${this.provider.id}`);
}
const servers = await this.provider.serverProvider.provideJupyterServers(token);
const servers = await Promise.resolve(this.provider.serverProvider.provideJupyterServers(token));
this._servers.clear();
servers.forEach((s) => this._servers.set(s.id, s));
(servers || []).forEach((s) => this._servers.set(s.id, s));
return servers;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/kernels/jupyter/finder/remoteKernelFinderController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ export class RemoteKernelFinderController implements IRemoteKernelFinderControll
}
const token = new CancellationTokenSource();
try {
const servers = await serverProvider.provideJupyterServers(token.token);
servers.forEach((server) => {
const servers = await Promise.resolve(serverProvider.provideJupyterServers(token.token));
(servers || []).forEach((server) => {
const serverProviderHandle = {
extensionId: collection.extensionId,
handle: server.id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,9 @@ export class RemoteNotebookKernelSourceSelector implements IRemoteNotebookKernel
(p) => p.extensionId === provider.extensionId && p.id === provider.id
)?.serverProvider;
const serversPromise = serverProvider?.provideJupyterServers
? serverProvider.provideJupyterServers.bind(serverProvider)(token)
? Promise.resolve(serverProvider.provideJupyterServers.bind(serverProvider)(token)).then(
(servers) => servers || []
)
: Promise.resolve([]);
const handledServerIds = new Set<string>();
const [jupyterServers] = await Promise.all([
Expand Down
4 changes: 2 additions & 2 deletions src/notebooks/controllers/remoteKernelControllerWatcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ export class RemoteKernelControllerWatcher implements IExtensionSyncActivationSe
const tokenSource = new CancellationTokenSource();
this.disposables.push(tokenSource);
try {
const currentServers = await serverProvider.provideJupyterServers(tokenSource.token);
const currentServers = await Promise.resolve(serverProvider.provideJupyterServers(tokenSource.token));
await this.removeControllersAndUriStorageBelongingToInvalidServers(
collection.extensionId,
collection.id,
currentServers.map((s) => s.id)
(currentServers || []).map((s) => s.id)
);
} finally {
tokenSource.dispose();
Expand Down
10 changes: 3 additions & 7 deletions src/standalone/api/api.jupyterProvider.vscode.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,13 +348,9 @@ suite('Jupyter Provider Tests', function () {
);

// Add a command and that command should be displayed along with the 2 servers.
let commandsToReturn = [{ label: 'Sample Command' }];
collection.commandProvider = {
commands: [
{
title: 'Sample Command'
}
],
provideCommands: () => Promise.resolve([]),
provideCommands: () => Promise.resolve(commandsToReturn),
handleCommand: () => Promise.resolve(undefined)
};
// await sleep(100);
Expand All @@ -374,7 +370,7 @@ suite('Jupyter Provider Tests', function () {
);

// Remove the command and try again.
collection.commandProvider.commands = [];
commandsToReturn = [];

const controllerId4 = await commands.executeCommand(actionCommand.command, ...(actionCommand.arguments || []));
// Verify a controller is not selected
Expand Down
Loading