Skip to content

Commit

Permalink
Support remote commands in the kernel picker (#10602)
Browse files Browse the repository at this point in the history
* Add commands and fixup the server selector

* Filter list of controllers by connection type

* Wire up commands

* Update telemetry

* Add another command for when already remote

* Make sure activation failures don't stop other activations

* Some more parts working

* Using multistep to setup back button and filtering

* Update telemetry

* Fix showing duplicates in custom quick pick

* Fix localization and refreshing between local and remote

* First try at a test

* Remove ability to set local launch independent of URI

* Fix back button

* Get all tests passing

* Update telemetry

* Add news entries

* Fix isLocalLaunch to actually use the internal flag

* Fix unit tests and use serverId

* Update telemetry

* Fix bug in load order

* Update telemetry
  • Loading branch information
rchiodo authored Jun 30, 2022
1 parent b1e9cbe commit e92cac6
Show file tree
Hide file tree
Showing 51 changed files with 1,292 additions and 482 deletions.
202 changes: 101 additions & 101 deletions TELEMETRY.md

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions news/1 Enhancements/10435.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rework kernel selection to be in either 'remote' mode or 'local' mode to avoid confusion about what kernels should be displayed.
1 change: 1 addition & 0 deletions news/2 Fixes/10191.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes problem where clipboard permissions are required in order to enter a Jupyter server URL.
1 change: 1 addition & 0 deletions news/2 Fixes/10363.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix problem of determining whether or not in 'local' or 'remote' mode for a jupyer connection.
40 changes: 38 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -841,11 +841,23 @@
},
{
"command": "jupyter.installPythonExtensionViaKernelPicker",
"title": "Install Python Extension"
"title": "%DataScience.installPythonExtensionViaKernelPickerTitle%"
},
{
"command": "jupyter.installPythonViaKernelPicker",
"title": "Install Python"
"title": "%DataScience.installPythonTitle%"
},
{
"command": "jupyter.switchToLocalKernels",
"title": "%DataScience.switchToLocalKernelsTitle%"
},
{
"command": "jupyter.switchToRemoteKernels",
"title": "%DataScience.switchToRemoteKernelsTitle%"
},
{
"command": "jupyter.switchToAnotherRemoteKernels",
"title": "%DataScience.switchToAnotherRemoteKernelsTitle%"
}
],
"menus": {
Expand Down Expand Up @@ -1007,6 +1019,18 @@
{
"command": "jupyter.installPythonViaKernelPicker",
"when": "jupyter.showInstallPythonCommand"
},
{
"command": "jupyter.switchToLocalKernels",
"when": "jupyter.showingRemoteNotWeb"
},
{
"command": "jupyter.switchToRemoteKernels",
"when": "jupyter.showingLocalOrWebEmpty"
},
{
"command": "jupyter.switchToAnotherRemoteKernels",
"when": "jupyter.showingRemoteKernels"
}
],
"interactive/toolbar": [
Expand Down Expand Up @@ -1089,6 +1113,18 @@
"command": "jupyter.installPythonViaKernelPicker",
"when": "false"
},
{
"command": "jupyter.switchToLocalKernels",
"when": "false"
},
{
"command": "jupyter.switchToRemoteKernels",
"when": "false"
},
{
"command": "jupyter.switchToAnotherRemoteKernels",
"when": "false"
},
{
"command": "jupyter.replayPylanceLog",
"title": "Replay Pylance Log",
Expand Down
6 changes: 5 additions & 1 deletion package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,10 @@
"Products.installingModule": "Installing {0}",
"DataScience.webNotSupported": "Operation not supported in web version of Jupyter Extension.",
"DataScience.outputSizeExceedLimit": "Output exceeds the <a href={0}>size limit</a>. Open the full output data <a href={1}>in a text editor</a>",
"DataScience.installPythonTitle": "Install Python",
"DataScience.installPythonExtensionViaKernelPickerTitle": "Install Python Extension",
"DataScience.switchToLocalKernelsTitle": "Connect to Local Kernels",
"DataScience.switchToRemoteKernelsTitle": "Connect to Your Own Jupyter Server",
"DataScience.switchToAnotherRemoteKernelsTitle": "Connect to Another Jupyter Server",
"DataScience.failedToInstallPythonExtension": "Failed to install the Python Extension."
}

3 changes: 3 additions & 0 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,4 +194,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
[DSCommands.ReplayPylanceLogStep]: [];
[DSCommands.InstallPythonExtensionViaKernelPicker]: [];
[DSCommands.InstallPythonViaKernelPicker]: [];
[DSCommands.SwitchToLocalKernels]: [];
[DSCommands.SwitchToRemoteKernels]: [];
[DSCommands.SwitchToAnotherRemoteKernels]: [];
}
6 changes: 3 additions & 3 deletions src/kernels/jupyter/jupyterConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ import { RemoteJupyterServerUriProviderError } from '../errors/remoteJupyterServ
import { BaseError } from '../../platform/errors/types';
import { IJupyterConnection } from '../types';
import { computeServerId, createRemoteConnectionInfo, extractJupyterServerHandleAndId } from './jupyterUtils';
import { ServerConnectionType } from './launcher/serverConnectionType';
import {
IJupyterServerUri,
IJupyterServerUriStorage,
IJupyterSessionManager,
IJupyterSessionManagerFactory,
IJupyterUriProviderRegistration
IJupyterUriProviderRegistration,
IServerConnectionType
} from './types';

@injectable()
Expand All @@ -29,7 +29,7 @@ export class JupyterConnection implements IExtensionSyncActivationService {
private readonly jupyterSessionManagerFactory: IJupyterSessionManagerFactory,
@inject(IDisposableRegistry)
private readonly disposables: IDisposableRegistry,
@inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType,
@inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType,
@inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage
) {
disposables.push(this);
Expand Down
4 changes: 2 additions & 2 deletions src/kernels/jupyter/launcher/commandLineSelector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ export class JupyterCommandLineSelector {
}

@captureTelemetry(Telemetry.SelectJupyterURI)
public selectJupyterCommandLine(file: Uri): Promise<void> {
public async selectJupyterCommandLine(file: Uri): Promise<void> {
const multiStep = this.multiStepFactory.create<{}>();
return multiStep.run(this.startSelectingCommandLine.bind(this, file), {});
await multiStep.run(this.startSelectingCommandLine.bind(this, file), {});
}

private async onDidChangeConfiguration(e: ConfigurationChangeEvent) {
Expand Down
5 changes: 2 additions & 3 deletions src/kernels/jupyter/launcher/notebookProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ import {
import { Cancellation } from '../../../platform/common/cancellation';
import { DisplayOptions } from '../../displayOptions';
import { IRawNotebookProvider } from '../../raw/types';
import { IJupyterNotebookProvider } from '../types';
import { ServerConnectionType } from './serverConnectionType';
import { IJupyterNotebookProvider, IServerConnectionType } from '../types';
import { sendKernelTelemetryWhenDone } from '../../telemetry/sendKernelTelemetryEvent';

@injectable()
Expand All @@ -33,7 +32,7 @@ export class NotebookProvider implements INotebookProvider {
@inject(IJupyterNotebookProvider)
private readonly jupyterNotebookProvider: IJupyterNotebookProvider,
@inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker,
@inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType
@inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType
) {}

// Attempt to connect to our server provider, and if we do, return the connection info
Expand Down
41 changes: 0 additions & 41 deletions src/kernels/jupyter/launcher/serverConnectionType.ts

This file was deleted.

58 changes: 45 additions & 13 deletions src/kernels/jupyter/launcher/serverUriStorage.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,30 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.
import { inject, injectable, named } from 'inversify';
import { EventEmitter, Memento } from 'vscode';
import { Event, EventEmitter, Memento } from 'vscode';
import {
IWorkspaceService,
IEncryptedStorage,
IApplicationEnvironment
} from '../../../platform/common/application/types';
import { Settings } from '../../../platform/common/constants';
import { getFilePath } from '../../../platform/common/platform/fs-paths';
import { ICryptoUtils, IMemento, GLOBAL_MEMENTO } from '../../../platform/common/types';
import { IJupyterServerUriStorage } from '../types';
import { ServerConnectionType } from './serverConnectionType';
import { ICryptoUtils, IMemento, GLOBAL_MEMENTO, IsWebExtension } from '../../../platform/common/types';
import { computeServerId } from '../jupyterUtils';
import { IJupyterServerUriStorage, IServerConnectionType } from '../types';

export const mementoKeyToIndicateIfConnectingToLocalKernelsOnly = 'connectToLocalKernelsOnly';
export const currentServerHashKey = 'currentServerHash';

/**
* Class for storing Jupyter Server URI values
*/
@injectable()
export class JupyterServerUriStorage implements IJupyterServerUriStorage {
export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServerConnectionType {
private lastSavedList?: Promise<{ uri: string; time: number; displayName?: string | undefined }[]>;
private currentUriPromise: Promise<string> | undefined;
private _currentServerId: string | undefined;
private _localOnly: boolean = false;
private _onDidChangeUri = new EventEmitter<void>();
public get onDidChangeUri() {
return this._onDidChangeUri.event;
Expand All @@ -28,14 +33,29 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage {
public get onDidRemoveUris() {
return this._onDidRemoveUris.event;
}
public get currentServerId(): string | undefined {
return this._currentServerId;
}
public get onDidChange(): Event<void> {
return this._onDidChangeUri.event;
}
public get isLocalLaunch(): boolean {
return this._localOnly;
}
constructor(
@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService,
@inject(ICryptoUtils) private readonly crypto: ICryptoUtils,
@inject(IEncryptedStorage) private readonly encryptedStorage: IEncryptedStorage,
@inject(IApplicationEnvironment) private readonly appEnv: IApplicationEnvironment,
@inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalMemento: Memento,
@inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType
@inject(IsWebExtension) readonly isWebExtension: boolean
) {
// Remember if local only
this._localOnly = isWebExtension
? false
: this.globalMemento.get<boolean>(mementoKeyToIndicateIfConnectingToLocalKernelsOnly, true);
this._currentServerId = this.globalMemento.get<string | undefined>(currentServerHashKey, undefined);

// Cache our current state so we don't keep asking for it from the encrypted storage
this.getUri().ignoreErrors();
}
Expand Down Expand Up @@ -182,33 +202,45 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage {
await this.setUri(Settings.JupyterServerLocalLaunch);
}
public async setUriToRemote(uri: string, displayName: string): Promise<void> {
await this.setUri(uri);
// Make sure to add to the saved list before we set the uri. Otherwise
// handlers for the URI changing will use the saved list to make sure the
// server id matches
await this.addToUriList(uri, Date.now(), displayName);
await this.setUri(uri);
}

public async setUri(uri: string) {
// Set the URI as our current state
this.currentUriPromise = Promise.resolve(uri);
if (uri === Settings.JupyterServerLocalLaunch) {
await this.serverConnectionType.setIsLocalLaunch(true);
} else {
this._currentServerId = computeServerId(uri);
this._localOnly = uri === Settings.JupyterServerLocalLaunch || uri === undefined;
this._onDidChangeUri.fire(); // Needs to happen as soon as we change so that dependencies update synchronously

// No update the async parts
await this.globalMemento.update(mementoKeyToIndicateIfConnectingToLocalKernelsOnly, this._localOnly);
await this.globalMemento.update(currentServerHashKey, this._currentServerId);

if (!this._localOnly) {
await this.addToUriList(uri, Date.now(), uri);
await this.serverConnectionType.setIsLocalLaunch(false);

// Save in the storage (unique account per workspace)
const key = this.getUriAccountKey();
await this.encryptedStorage.store(Settings.JupyterServerRemoteLaunchService, key, uri);
}
this._onDidChangeUri.fire();
}
private async getUriInternal(): Promise<string> {
if (this.serverConnectionType.isLocalLaunch) {
if (this.isLocalLaunch) {
return Settings.JupyterServerLocalLaunch;
} else {
// Should be stored in encrypted storage based on the workspace
const key = this.getUriAccountKey();
const storedUri = await this.encryptedStorage.retrieve(Settings.JupyterServerRemoteLaunchService, key);

// Update server id if not already set
if (!this._currentServerId && storedUri) {
this._currentServerId = computeServerId(storedUri);
}

return storedUri || Settings.JupyterServerLocalLaunch;
}
}
Expand Down
Loading

0 comments on commit e92cac6

Please sign in to comment.