diff --git a/TELEMETRY.md b/TELEMETRY.md index 08da62f2383..7894d458113 100644 --- a/TELEMETRY.md +++ b/TELEMETRY.md @@ -898,13 +898,13 @@ No properties for event [src/kernels/jupyter/serverSelector.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/serverSelector.ts) ```typescript - return multiStep.run(this.startSelectingURI.bind(this, allowLocal), {}); + await this.serverUriStorage.setUriToLocal(); } @captureTelemetry(Telemetry.EnterJupyterURI) @traceDecoratorError('Failed to enter Jupyter Uri') - public async enterJupyterURI(): Promise { - let initialValue = defaultUri; + public async setJupyterURIToRemote(userURI: string, ignoreValidation?: boolean): Promise { + // Double check this server can be connected to. Might need a password, might need a allowUnauthorized ``` @@ -927,24 +927,24 @@ No properties for event [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - properties?: P[E] & { waitBeforeSending?: Promise }, - ex?: Error -) { - if (eventName === Telemetry.ExecuteCell) { - setSharedProperty('userExecutedCell', 'true'); - } + properties?: P[E] & { waitBeforeSending?: Promise }, + ex?: Error +) { + if (eventName === Telemetry.ExecuteCell) { + setSharedProperty('userExecutedCell', 'true'); + } ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - stopWatch?: StopWatch, - properties?: P[E] & { [waitBeforeSending]?: Promise } -) { - if (eventName === Telemetry.ExecuteCell) { - setSharedProperty('userExecutedCell', 'true'); - } + stopWatch?: StopWatch, + properties?: P[E] & { [waitBeforeSending]?: Promise } +) { + if (eventName === Telemetry.ExecuteCell) { + setSharedProperty('userExecutedCell', 'true'); + } // eslint-disable-next-line @typescript-eslint/no-explicit-any ``` @@ -2655,36 +2655,36 @@ No properties for event [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function resetData(resource: Resource, eventName: string, properties: any) { - // Once we have successfully interrupted, clear the interrupt counter. - if (eventName === Telemetry.NotebookInterrupt) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookInterrupt] = properties; +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function resetData(resource: Resource, eventName: string, properties: any) { + // Once we have successfully interrupted, clear the interrupt counter. + if (eventName === Telemetry.NotebookInterrupt) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookInterrupt] = properties; // Check result to determine if success. ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript -function resetData(resource: Resource, eventName: string, properties: any) { - // Once we have successfully interrupted, clear the interrupt counter. - if (eventName === Telemetry.NotebookInterrupt) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookInterrupt] = properties; - // Check result to determine if success. +function resetData(resource: Resource, eventName: string, properties: any) { + // Once we have successfully interrupted, clear the interrupt counter. + if (eventName === Telemetry.NotebookInterrupt) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookInterrupt] = properties; + // Check result to determine if success. if (data && 'result' in data && data.result === InterruptResult.Success) { ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - // Once we have successfully interrupted, clear the interrupt counter. - if (eventName === Telemetry.NotebookInterrupt) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookInterrupt] = properties; - // Check result to determine if success. - if (data && 'result' in data && data.result === InterruptResult.Success) { + // Once we have successfully interrupted, clear the interrupt counter. + if (eventName === Telemetry.NotebookInterrupt) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookInterrupt] = properties; + // Check result to determine if success. + if (data && 'result' in data && data.result === InterruptResult.Success) { clearInterruptCounter(resource); ``` @@ -2736,12 +2736,12 @@ No properties for event [src/notebooks/telemetry/notebookOrKernelLanguageTelemetry.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/telemetry/notebookOrKernelLanguageTelemetry.ts) ```typescript -import { getTelemetrySafeLanguage } from '../../platform/telemetry/helpers'; - -export function sendNotebookOrKernelLanguageTelemetry( - telemetryEvent: Telemetry.SwitchToExistingKernel | Telemetry.NotebookLanguage, - language?: string -) { +import { getTelemetrySafeLanguage } from '../../platform/telemetry/helpers'; + +export function sendNotebookOrKernelLanguageTelemetry( + telemetryEvent: Telemetry.SwitchToExistingKernel | Telemetry.NotebookLanguage, + language?: string +) { language = getTelemetrySafeLanguage(language); ``` @@ -2764,36 +2764,36 @@ No properties for event [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - } - } - // Once we have successfully restarted, clear the interrupt counter. - if (eventName === Telemetry.NotebookRestart) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookRestart] = properties; + } + } + // Once we have successfully restarted, clear the interrupt counter. + if (eventName === Telemetry.NotebookRestart) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookRestart] = properties; // For restart to be successful, we should not have `failed` ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - } - // Once we have successfully restarted, clear the interrupt counter. - if (eventName === Telemetry.NotebookRestart) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookRestart] = properties; - // For restart to be successful, we should not have `failed` + } + // Once we have successfully restarted, clear the interrupt counter. + if (eventName === Telemetry.NotebookRestart) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookRestart] = properties; + // For restart to be successful, we should not have `failed` const failed = data && 'failed' in data ? data.failed : false; ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - // Once we have successfully restarted, clear the interrupt counter. - if (eventName === Telemetry.NotebookRestart) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookRestart] = properties; - // For restart to be successful, we should not have `failed` - const failed = data && 'failed' in data ? data.failed : false; + // Once we have successfully restarted, clear the interrupt counter. + if (eventName === Telemetry.NotebookRestart) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookRestart] = properties; + // For restart to be successful, we should not have `failed` + const failed = data && 'failed' in data ? data.failed : false; if (!failed) { ``` @@ -2840,36 +2840,36 @@ No properties for event [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript - -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function incrementStartFailureCount(resource: Resource, eventName: any, properties: any) { - if (eventName === Telemetry.NotebookStart) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookStart] = properties; + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function incrementStartFailureCount(resource: Resource, eventName: any, properties: any) { + if (eventName === Telemetry.NotebookStart) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookStart] = properties; // Check start failed. ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function incrementStartFailureCount(resource: Resource, eventName: any, properties: any) { - if (eventName === Telemetry.NotebookStart) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookStart] = properties; - // Check start failed. +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function incrementStartFailureCount(resource: Resource, eventName: any, properties: any) { + if (eventName === Telemetry.NotebookStart) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookStart] = properties; + // Check start failed. if (data && 'failed' in data && data.failed) { ``` [src/kernels/telemetry/sendKernelTelemetryEvent.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/telemetry/sendKernelTelemetryEvent.ts) ```typescript -function incrementStartFailureCount(resource: Resource, eventName: any, properties: any) { - if (eventName === Telemetry.NotebookStart) { - let kv: Pick; - const data: undefined | typeof kv[Telemetry.NotebookStart] = properties; - // Check start failed. - if (data && 'failed' in data && data.failed) { +function incrementStartFailureCount(resource: Resource, eventName: any, properties: any) { + if (eventName === Telemetry.NotebookStart) { + let kv: Pick; + const data: undefined | typeof kv[Telemetry.NotebookStart] = properties; + // Check start failed. + if (data && 'failed' in data && data.failed) { trackKernelResourceInformation(resource, { startFailed: true }); ``` @@ -4038,9 +4038,9 @@ No properties for event ) {} @captureTelemetry(Telemetry.SelectJupyterURI) - @traceDecoratorError('Failed to select Jupyter Uri') public selectJupyterURI( - allowLocal: boolean, + commandSource: SelectJupyterUriCommandSource = 'nonUser', + existingMultiStep?: IMultiStepInput<{}> ``` @@ -4050,9 +4050,9 @@ No properties for event } @captureTelemetry(Telemetry.SelectJupyterURI) - public selectJupyterCommandLine(file: Uri): Promise { + public async selectJupyterCommandLine(file: Uri): Promise { const multiStep = this.multiStepFactory.create<{}>(); - return multiStep.run(this.startSelectingCommandLine.bind(this, file), {}); + await multiStep.run(this.startSelectingCommandLine.bind(this, file), {}); ``` @@ -4258,9 +4258,9 @@ No properties for event [src/kernels/jupyter/serverSelector.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/serverSelector.ts) ```typescript - return computeServerId(uri); } } + @captureTelemetry(Telemetry.SetJupyterURIToLocal) public async setJupyterURIToLocal(): Promise { await this.serverUriStorage.setUriToLocal(); @@ -4288,13 +4288,13 @@ No properties for event [src/kernels/jupyter/serverSelector.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/serverSelector.ts) ```typescript - allowLocal: boolean, - commandSource: SelectJupyterUriCommandSource = 'nonUser' - ): Promise { + commandSource: SelectJupyterUriCommandSource = 'nonUser', + existingMultiStep?: IMultiStepInput<{}> + ): Promise | void> { sendTelemetryEvent(Telemetry.SetJupyterURIUIDisplayed, undefined, { commandSource }); - const multiStep = this.multiStepFactory.create<{}>(); + if (existingMultiStep) { ``` @@ -7179,10 +7179,10 @@ No description provided ## Locations Used -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript - - if (this.extensionChecker.isPythonExtensionInstalled) { + // Make sure that we didn't timeout waiting for the hook + if (this.extensionChecker.isPythonExtensionInstalled && typeof hookResult !== 'number') { traceInfo('Python Extension installed via Kernel Picker command'); sendTelemetryEvent(Telemetry.PythonExtensionInstalledViaKernelPicker, undefined, { action: 'success' @@ -7191,7 +7191,7 @@ No description provided ``` -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript await this.controllerLoader.loadControllers(true); } else { @@ -7199,7 +7199,7 @@ No description provided sendTelemetryEvent(Telemetry.PythonExtensionInstalledViaKernelPicker, undefined, { action: 'failed' }); - throw new Error('Failed to install Python Extension via Kernel Picker command'); + this.errorHandler ``` @@ -7257,7 +7257,7 @@ No description provided ``` -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript // click run again private async installPythonExtensionViaKernelPicker(): Promise { @@ -7269,7 +7269,7 @@ No description provided ``` -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript Common.install() ); @@ -7281,7 +7281,7 @@ No description provided ``` -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript return true; } else { @@ -7546,7 +7546,7 @@ No description provided ## Locations Used -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript // Unlike installing the python extension we don't expect in progress executions to be handled // when this command is installed, user will have to manually install python and rerun the cell @@ -7558,7 +7558,7 @@ No description provided ``` -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript ); @@ -7570,7 +7570,7 @@ No description provided ``` -[src/notebooks/controllers/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/installPythonControllerCommands.ts) +[src/notebooks/controllers/commands/installPythonControllerCommands.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/controllers/commands/installPythonControllerCommands.ts) ```typescript // Direct the user to download from python.org this.appShell.openUrl('https://www.python.org/downloads'); @@ -8622,12 +8622,12 @@ No description provided [src/notebooks/telemetry/notebookOrKernelLanguageTelemetry.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/notebooks/telemetry/notebookOrKernelLanguageTelemetry.ts) ```typescript -import { getTelemetrySafeLanguage } from '../../platform/telemetry/helpers'; - -export function sendNotebookOrKernelLanguageTelemetry( - telemetryEvent: Telemetry.SwitchToExistingKernel | Telemetry.NotebookLanguage, - language?: string -) { +import { getTelemetrySafeLanguage } from '../../platform/telemetry/helpers'; + +export function sendNotebookOrKernelLanguageTelemetry( + telemetryEvent: Telemetry.SwitchToExistingKernel | Telemetry.NotebookLanguage, + language?: string +) { language = getTelemetrySafeLanguage(language); ``` diff --git a/news/1 Enhancements/10435.md b/news/1 Enhancements/10435.md new file mode 100644 index 00000000000..cf36d067142 --- /dev/null +++ b/news/1 Enhancements/10435.md @@ -0,0 +1 @@ +Rework kernel selection to be in either 'remote' mode or 'local' mode to avoid confusion about what kernels should be displayed. diff --git a/news/2 Fixes/10191.md b/news/2 Fixes/10191.md new file mode 100644 index 00000000000..76ae60e0135 --- /dev/null +++ b/news/2 Fixes/10191.md @@ -0,0 +1 @@ +Fixes problem where clipboard permissions are required in order to enter a Jupyter server URL. diff --git a/news/2 Fixes/10363.md b/news/2 Fixes/10363.md new file mode 100644 index 00000000000..486091dcec2 --- /dev/null +++ b/news/2 Fixes/10363.md @@ -0,0 +1 @@ +Fix problem of determining whether or not in 'local' or 'remote' mode for a jupyer connection. diff --git a/package.json b/package.json index ee869ece32f..bd7292bd5e7 100644 --- a/package.json +++ b/package.json @@ -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": { @@ -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": [ @@ -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", diff --git a/package.nls.json b/package.nls.json index 8fb81264f73..c3d6609bb22 100644 --- a/package.nls.json +++ b/package.nls.json @@ -956,6 +956,10 @@ "Products.installingModule": "Installing {0}", "DataScience.webNotSupported": "Operation not supported in web version of Jupyter Extension.", "DataScience.outputSizeExceedLimit": "Output exceeds the size limit. Open the full output data in a text editor", + "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." } - diff --git a/src/commands.ts b/src/commands.ts index e9e74a049cc..e3fb53ac613 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -194,4 +194,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu [DSCommands.ReplayPylanceLogStep]: []; [DSCommands.InstallPythonExtensionViaKernelPicker]: []; [DSCommands.InstallPythonViaKernelPicker]: []; + [DSCommands.SwitchToLocalKernels]: []; + [DSCommands.SwitchToRemoteKernels]: []; + [DSCommands.SwitchToAnotherRemoteKernels]: []; } diff --git a/src/kernels/jupyter/jupyterConnection.ts b/src/kernels/jupyter/jupyterConnection.ts index 8b4e60e62b9..c9ee0ca2444 100644 --- a/src/kernels/jupyter/jupyterConnection.ts +++ b/src/kernels/jupyter/jupyterConnection.ts @@ -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() @@ -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); diff --git a/src/kernels/jupyter/launcher/commandLineSelector.ts b/src/kernels/jupyter/launcher/commandLineSelector.ts index bfc39ecc16c..1b76364b7a6 100644 --- a/src/kernels/jupyter/launcher/commandLineSelector.ts +++ b/src/kernels/jupyter/launcher/commandLineSelector.ts @@ -34,9 +34,9 @@ export class JupyterCommandLineSelector { } @captureTelemetry(Telemetry.SelectJupyterURI) - public selectJupyterCommandLine(file: Uri): Promise { + public async selectJupyterCommandLine(file: Uri): Promise { 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) { diff --git a/src/kernels/jupyter/launcher/notebookProvider.ts b/src/kernels/jupyter/launcher/notebookProvider.ts index 734434e4929..6ba6a65b1ba 100644 --- a/src/kernels/jupyter/launcher/notebookProvider.ts +++ b/src/kernels/jupyter/launcher/notebookProvider.ts @@ -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() @@ -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 diff --git a/src/kernels/jupyter/launcher/serverConnectionType.ts b/src/kernels/jupyter/launcher/serverConnectionType.ts deleted file mode 100644 index 1335ac3902c..00000000000 --- a/src/kernels/jupyter/launcher/serverConnectionType.ts +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -import { inject, injectable, named } from 'inversify'; -import { EventEmitter, Memento } from 'vscode'; -import { GLOBAL_MEMENTO, IMemento, IsWebExtension } from '../../../platform/common/types'; - -export const mementoKeyToIndicateIfConnectingToLocalKernelsOnly = 'connectToLocalKernelsOnly'; - -@injectable() -export class ServerConnectionType { - private _isLocalLaunch?: boolean; - private readonly _onDidChange = new EventEmitter(); - public readonly onDidChange = this._onDidChange.event; - constructor( - @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly memento: Memento, - @inject(IsWebExtension) private readonly isWebExtension: boolean - ) {} - - public get isLocalLaunch() { - if (this.isWebExtension) { - return false; - } - if (typeof this._isLocalLaunch === 'boolean') { - return this._isLocalLaunch; - } - const connectToLocalOnly = this.memento.get(mementoKeyToIndicateIfConnectingToLocalKernelsOnly, true); - if (typeof this._isLocalLaunch !== 'boolean') { - this._isLocalLaunch = connectToLocalOnly; - } - return connectToLocalOnly; - } - public async setIsLocalLaunch(localLaunch: boolean) { - if (this.isWebExtension) { - return; - } - this._isLocalLaunch = localLaunch; - this._onDidChange.fire(); - await this.memento.update(mementoKeyToIndicateIfConnectingToLocalKernelsOnly, localLaunch); - } -} diff --git a/src/kernels/jupyter/launcher/serverUriStorage.ts b/src/kernels/jupyter/launcher/serverUriStorage.ts index c0e2e7d0168..44edc9f44fc 100644 --- a/src/kernels/jupyter/launcher/serverUriStorage.ts +++ b/src/kernels/jupyter/launcher/serverUriStorage.ts @@ -1,7 +1,7 @@ // 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, @@ -9,17 +9,22 @@ import { } 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 | undefined; + private _currentServerId: string | undefined; + private _localOnly: boolean = false; private _onDidChangeUri = new EventEmitter(); public get onDidChangeUri() { return this._onDidChangeUri.event; @@ -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 { + 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(mementoKeyToIndicateIfConnectingToLocalKernelsOnly, true); + this._currentServerId = this.globalMemento.get(currentServerHashKey, undefined); + // Cache our current state so we don't keep asking for it from the encrypted storage this.getUri().ignoreErrors(); } @@ -182,33 +202,45 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage { await this.setUri(Settings.JupyterServerLocalLaunch); } public async setUriToRemote(uri: string, displayName: string): Promise { - 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 { - 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; } } diff --git a/src/kernels/jupyter/serverSelector.ts b/src/kernels/jupyter/serverSelector.ts index ea1be510a83..25320db9e27 100644 --- a/src/kernels/jupyter/serverSelector.ts +++ b/src/kernels/jupyter/serverSelector.ts @@ -5,8 +5,8 @@ import { inject, injectable } from 'inversify'; import isNil = require('lodash/isNil'); -import { EventEmitter, QuickPickItem, ThemeIcon, Uri } from 'vscode'; -import { IApplicationShell, IClipboard } from '../../platform/common/application/types'; +import { EventEmitter, QuickPickItem, ThemeIcon } from 'vscode'; +import { IApplicationShell } from '../../platform/common/application/types'; import { traceDecoratorError, traceError, traceWarning } from '../../platform/logging'; import { DataScience } from '../../platform/common/utils/localize'; import { @@ -37,10 +37,7 @@ import { JupyterSelfCertsError } from '../../platform/errors/jupyterSelfCertsErr import { RemoteJupyterServerConnectionError } from '../../platform/errors/remoteJupyterServerConnectionError'; import { JupyterSelfCertsExpiredError } from '../../platform/errors/jupyterSelfCertsExpiredError'; -const defaultUri = 'https://hostname:8080/?token=849d61a414abafab97bc4aab1f3547755ddc232c2b8cb7fe'; - interface ISelectUriQuickPickItem extends QuickPickItem { - newChoice: boolean; provider?: IJupyterUriProvider; url?: string; } @@ -54,11 +51,7 @@ export type SelectJupyterUriCommandSource = | 'prompt'; @injectable() export class JupyterServerSelector { - private readonly localLabel = `$(zap) ${DataScience.jupyterSelectURINoneLabel()}`; - private readonly newLabel = `$(server) ${DataScience.jupyterSelectURINewLabel()}`; - private readonly remoteLabel = `$(server) ${DataScience.jupyterSelectURIRemoteLabel()}`; constructor( - @inject(IClipboard) private readonly clipboard: IClipboard, @inject(IMultiStepInputFactory) private readonly multiStepFactory: IMultiStepInputFactory, @inject(IJupyterUriProviderRegistration) private extraUriProviders: IJupyterUriProviderRegistration, @@ -72,48 +65,28 @@ export class JupyterServerSelector { ) {} @captureTelemetry(Telemetry.SelectJupyterURI) - @traceDecoratorError('Failed to select Jupyter Uri') public selectJupyterURI( - allowLocal: boolean, - commandSource: SelectJupyterUriCommandSource = 'nonUser' - ): Promise { + commandSource: SelectJupyterUriCommandSource = 'nonUser', + existingMultiStep?: IMultiStepInput<{}> + ): Promise | void> { sendTelemetryEvent(Telemetry.SetJupyterURIUIDisplayed, undefined, { commandSource }); - const multiStep = this.multiStepFactory.create<{}>(); - return multiStep.run(this.startSelectingURI.bind(this, allowLocal), {}); - } - - @captureTelemetry(Telemetry.EnterJupyterURI) - @traceDecoratorError('Failed to enter Jupyter Uri') - public async enterJupyterURI(): Promise { - let initialValue = defaultUri; - try { - const text = await this.clipboard.readText().catch(() => ''); - const parsedUri = Uri.parse(text.trim(), true); - // Only display http/https uris. - initialValue = text && parsedUri && parsedUri.scheme.toLowerCase().startsWith('http') ? text : defaultUri; - } catch { - // We can ignore errors. - } - // Ask the user to enter a URI to connect to. - const uri = await this.applicationShell.showInputBox({ - title: DataScience.jupyterSelectURIPrompt(), - value: initialValue || defaultUri, - validateInput: this.validateSelectJupyterURI, - prompt: '' - }); - - if (uri) { - await this.setJupyterURIToRemote(uri, true); - return computeServerId(uri); + if (existingMultiStep) { + return this.startSelectingURI(existingMultiStep, {}); + } else { + const multiStep = this.multiStepFactory.create<{}>(); + return multiStep.run(this.startSelectingURI.bind(this), {}); } } + @captureTelemetry(Telemetry.SetJupyterURIToLocal) public async setJupyterURIToLocal(): Promise { await this.serverUriStorage.setUriToLocal(); } + @captureTelemetry(Telemetry.EnterJupyterURI) + @traceDecoratorError('Failed to enter Jupyter Uri') public async setJupyterURIToRemote(userURI: string, ignoreValidation?: boolean): Promise { // Double check this server can be connected to. Might need a password, might need a allowUnauthorized try { @@ -154,18 +127,14 @@ export class JupyterServerSelector { }); } - private async startSelectingURI( - allowLocal: boolean, - input: IMultiStepInput<{}>, - _state: {} - ): Promise | void> { + private async startSelectingURI(input: IMultiStepInput<{}>, _state: {}): Promise | void> { // First step, show a quick pick to choose either the remote or the local. // newChoice element will be set if the user picked 'enter a new server' // Get the list of items and show what the current value is const remoteUri = await this.serverUriStorage.getRemoteUri(); - const items = await this.getUriPickList(allowLocal, remoteUri); - const activeItem = items.find((i) => i.url === remoteUri || (i.label === this.localLabel && !remoteUri)); + const items = await this.getUriPickList(remoteUri); + const activeItem = items.find((i) => i.url === remoteUri); const currentValue = !remoteUri ? DataScience.jupyterSelectURINoneLabel() : activeItem?.label; const placeholder = currentValue // This will show at the top (current value really) ? DataScience.jupyterSelectURIQuickPickCurrent().format(currentValue) @@ -177,9 +146,9 @@ export class JupyterServerSelector { placeholder, items, activeItem, - title: allowLocal - ? DataScience.jupyterSelectURIQuickPickTitle() - : DataScience.jupyterSelectURIQuickPickTitleRemoteOnly(), + acceptFilterBoxTextAsSelection: true, + validate: this.validateSelectJupyterURI.bind(this), + title: DataScience.jupyterSelectURIQuickPickTitle(), onDidTriggerItemButton: (e) => { const url = e.item.url; if (url && e.button.tooltip === DataScience.removeRemoteJupyterServerEntryInQuickPick()) { @@ -188,17 +157,17 @@ export class JupyterServerSelector { ); items.splice(items.indexOf(e.item), 1); onDidChangeItems.fire(items.concat([])); + } else if (e.button) { + throw InputFlowAction.back; } }, onDidChangeItems: onDidChangeItems.event }); await pendingUpdatesToUri.catch((ex) => traceError('Failed to update Uri list', ex)); - if (item.label === this.localLabel) { - await this.setJupyterURIToLocal(); - } else if (!item.newChoice && !item.provider) { - await this.setJupyterURIToRemote(!isNil(item.url) ? item.url : item.label); + if (typeof item === 'string') { + await this.setJupyterURIToRemote(item); } else if (!item.provider) { - return this.selectRemoteURI.bind(this); + await this.setJupyterURIToRemote(!isNil(item.url) ? item.url : item.label); } else { return this.selectProviderURI.bind(this, item.provider, item); } @@ -227,28 +196,6 @@ export class JupyterServerSelector { await this.setJupyterURIToRemote(uri); } } - private async selectRemoteURI(input: IMultiStepInput<{}>, _state: {}): Promise | void> { - let initialValue = defaultUri; - try { - const text = await this.clipboard.readText().catch(() => ''); - const parsedUri = Uri.parse(text.trim(), true); - // Only display http/https uris. - initialValue = text && parsedUri && parsedUri.scheme.toLowerCase().startsWith('http') ? text : defaultUri; - } catch { - // We can ignore errors. - } - // Ask the user to enter a URI to connect to. - const uri = await input.showInputBox({ - title: DataScience.jupyterSelectURIPrompt(), - value: initialValue || defaultUri, - validate: this.validateSelectJupyterURI, - prompt: '' - }); - - if (uri) { - await this.setJupyterURIToRemote(uri, true); - } - } public validateSelectJupyterURI = async (inputText: string): Promise => { inputText = inputText.trim(); @@ -291,7 +238,7 @@ export class JupyterServerSelector { } }; - private async getUriPickList(allowLocal: boolean, currentRemoteUri?: string): Promise { + private async getUriPickList(currentRemoteUri?: string): Promise { // Ask our providers to stick on items let providerItems: ISelectUriQuickPickItem[] = []; const providers = await this.extraUriProviders.getProviders(); @@ -307,20 +254,7 @@ export class JupyterServerSelector { }); } - // Always have 'local' and 'add new' - let items: ISelectUriQuickPickItem[] = []; - if (allowLocal) { - items.push({ label: this.localLabel, detail: DataScience.jupyterSelectURINoneDetail(), newChoice: false }); - items = items.concat(providerItems); - items.push({ label: this.newLabel, detail: DataScience.jupyterSelectURINewDetail(), newChoice: true }); - } else { - items = items.concat(providerItems); - items.push({ - label: this.remoteLabel, - detail: DataScience.jupyterSelectURIRemoteDetail(), - newChoice: true - }); - } + let items: ISelectUriQuickPickItem[] = [...providerItems]; // Get our list of recent server connections and display that as well const savedURIList = await this.serverUriStorage.getSavedUriList(); @@ -331,7 +265,6 @@ export class JupyterServerSelector { items.push({ label: !isNil(uriItem.displayName) ? uriItem.displayName : uriItem.uri, detail: DataScience.jupyterSelectURIMRUDetail().format(uriDate.toLocaleString()), - newChoice: false, url: uriItem.uri, buttons: isSelected ? [] // Cannot delete the current Uri (you can only switch to local). diff --git a/src/kernels/jupyter/serviceRegistry.node.ts b/src/kernels/jupyter/serviceRegistry.node.ts index 9addf7acd30..cff16654350 100644 --- a/src/kernels/jupyter/serviceRegistry.node.ts +++ b/src/kernels/jupyter/serviceRegistry.node.ts @@ -35,7 +35,6 @@ import { HostJupyterServerFactory } from './launcher/liveshare/hostJupyterServer import { NotebookProvider } from './launcher/notebookProvider'; import { NotebookServerProvider } from './launcher/notebookServerProvider'; import { NotebookStarter } from './launcher/notebookStarter.node'; -import { ServerConnectionType } from './launcher/serverConnectionType'; import { JupyterServerUriStorage } from './launcher/serverUriStorage'; import { LiveRemoteKernelConnectionUsageTracker } from './liveRemoteKernelConnectionTracker'; import { RemoteKernelFinder } from './remoteKernelFinder'; @@ -62,7 +61,8 @@ import { IJupyterRequestAgentCreator, INotebookServerFactory, ILiveRemoteKernelConnectionUsageTracker, - IJupyterRemoteCachedKernelValidator + IJupyterRemoteCachedKernelValidator, + IServerConnectionType } from './types'; import { IJupyterCommandFactory, IJupyterSubCommandExecutionService } from './types.node'; @@ -128,6 +128,7 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole JupyterUriProviderRegistration ); serviceManager.addSingleton(IJupyterServerUriStorage, JupyterServerUriStorage); + serviceManager.addBinding(IJupyterServerUriStorage, IServerConnectionType); serviceManager.addSingleton(INotebookStarter, NotebookStarter); serviceManager.addSingleton(INotebookProvider, NotebookProvider); serviceManager.addSingleton(IJupyterBackingFileCreator, BackingFileCreator); @@ -138,7 +139,6 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole ); serviceManager.addSingleton(IJupyterRequestCreator, JupyterRequestCreator); serviceManager.addSingleton(IJupyterRequestAgentCreator, RequestAgentCreator); - serviceManager.addSingleton(ServerConnectionType, ServerConnectionType); serviceManager.addSingleton(JupyterConnection, JupyterConnection); serviceManager.addBinding(JupyterConnection, IExtensionSyncActivationService); serviceManager.addSingleton( diff --git a/src/kernels/jupyter/serviceRegistry.web.ts b/src/kernels/jupyter/serviceRegistry.web.ts index 5f556bc7cf9..94d2eb849fd 100644 --- a/src/kernels/jupyter/serviceRegistry.web.ts +++ b/src/kernels/jupyter/serviceRegistry.web.ts @@ -19,7 +19,6 @@ import { HostJupyterExecution } from './launcher/liveshare/hostJupyterExecution' import { HostJupyterServerFactory } from './launcher/liveshare/hostJupyterServerFactory'; import { NotebookProvider } from './launcher/notebookProvider'; import { NotebookServerProvider } from './launcher/notebookServerProvider'; -import { ServerConnectionType } from './launcher/serverConnectionType'; import { JupyterServerUriStorage } from './launcher/serverUriStorage'; import { LiveRemoteKernelConnectionUsageTracker } from './liveRemoteKernelConnectionTracker'; import { RemoteKernelFinder } from './remoteKernelFinder'; @@ -40,7 +39,8 @@ import { IJupyterRequestCreator, INotebookServerFactory, ILiveRemoteKernelConnectionUsageTracker, - IJupyterRemoteCachedKernelValidator + IJupyterRemoteCachedKernelValidator, + IServerConnectionType } from './types'; export function registerTypes(serviceManager: IServiceManager, _isDevMode: boolean) { @@ -61,6 +61,7 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole JupyterUriProviderRegistration ); serviceManager.addSingleton(IJupyterServerUriStorage, JupyterServerUriStorage); + serviceManager.addBinding(IJupyterServerUriStorage, IServerConnectionType); serviceManager.addSingleton(INotebookProvider, NotebookProvider); serviceManager.addSingleton(IJupyterBackingFileCreator, BackingFileCreator); serviceManager.addSingleton(IExtensionSingleActivationService, CommandRegistry); @@ -71,7 +72,6 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole ); serviceManager.addSingleton(IJupyterServerProvider, NotebookServerProvider); serviceManager.addSingleton(IJupyterRequestCreator, JupyterRequestCreator); - serviceManager.addSingleton(ServerConnectionType, ServerConnectionType); serviceManager.addSingleton(JupyterConnection, JupyterConnection); serviceManager.addBinding(JupyterConnection, IExtensionSyncActivationService); serviceManager.addSingleton( diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts index 05fa99849d8..c54449033b4 100644 --- a/src/kernels/jupyter/types.ts +++ b/src/kernels/jupyter/types.ts @@ -231,6 +231,7 @@ export interface IJupyterUriProviderRegistration { export const IJupyterServerUriStorage = Symbol('IJupyterServerUriStorage'); export interface IJupyterServerUriStorage { + readonly currentServerId: string | undefined; readonly onDidChangeUri: Event; readonly onDidRemoveUris: Event; addToUriList(uri: string, time: number, displayName: string): Promise; @@ -322,3 +323,10 @@ export const IJupyterRemoteCachedKernelValidator = Symbol('IJupyterRemoteCachedK export interface IJupyterRemoteCachedKernelValidator { isValid(kernel: LiveRemoteKernelConnectionMetadata): Promise; } + +export const IServerConnectionType = Symbol('IServerConnectionType'); + +export interface IServerConnectionType { + isLocalLaunch: boolean; + onDidChange: Event; +} diff --git a/src/kernels/kernelFinder.base.ts b/src/kernels/kernelFinder.base.ts index 3b7b2c01c7f..7838db1f045 100644 --- a/src/kernels/kernelFinder.base.ts +++ b/src/kernels/kernelFinder.base.ts @@ -16,8 +16,7 @@ import { captureTelemetry, sendTelemetryEvent } from '../telemetry'; import { DisplayOptions } from './displayOptions'; import { rankKernels, deserializeKernelConnection, serializeKernelConnection, isExactMatch } from './helpers'; import { computeServerId } from './jupyter/jupyterUtils'; -import { ServerConnectionType } from './jupyter/launcher/serverConnectionType'; -import { IJupyterServerUriStorage } from './jupyter/types'; +import { IJupyterServerUriStorage, IServerConnectionType } from './jupyter/types'; import { PreferredRemoteKernelIdProvider } from './jupyter/preferredRemoteKernelIdProvider'; import { ILocalKernelFinder, IRemoteKernelFinder } from './raw/types'; import { @@ -44,7 +43,7 @@ export abstract class BaseKernelFinder implements IKernelFinder { private readonly remoteKernelFinder: IRemoteKernelFinder | undefined, private readonly globalState: Memento, protected readonly serverUriStorage: IJupyterServerUriStorage, - protected readonly serverConnectionType: ServerConnectionType + protected readonly serverConnectionType: IServerConnectionType ) {} @traceDecoratorVerbose('Rank Kernels', TraceOptions.BeforeCall | TraceOptions.Arguments) diff --git a/src/kernels/kernelFinder.node.ts b/src/kernels/kernelFinder.node.ts index e4c6b1af279..b279becf780 100644 --- a/src/kernels/kernelFinder.node.ts +++ b/src/kernels/kernelFinder.node.ts @@ -4,8 +4,7 @@ import { injectable, inject, named } from 'inversify'; import { Memento } from 'vscode'; import { IFileSystemNode } from '../platform/common/platform/types.node'; import { GLOBAL_MEMENTO, IMemento } from '../platform/common/types'; -import { ServerConnectionType } from './jupyter/launcher/serverConnectionType'; -import { IJupyterRemoteCachedKernelValidator, IJupyterServerUriStorage } from './jupyter/types'; +import { IJupyterRemoteCachedKernelValidator, IJupyterServerUriStorage, IServerConnectionType } from './jupyter/types'; import { BaseKernelFinder } from './kernelFinder.base'; import { PreferredRemoteKernelIdProvider } from './jupyter/preferredRemoteKernelIdProvider'; import { ILocalKernelFinder, IRemoteKernelFinder } from './raw/types'; @@ -21,7 +20,7 @@ export class KernelFinder extends BaseKernelFinder { @inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento, @inject(IFileSystemNode) private readonly fs: IFileSystemNode, @inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage, - @inject(ServerConnectionType) serverConnectionType: ServerConnectionType, + @inject(IServerConnectionType) serverConnectionType: IServerConnectionType, @inject(IJupyterRemoteCachedKernelValidator) protected readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator ) { diff --git a/src/kernels/kernelFinder.web.ts b/src/kernels/kernelFinder.web.ts index 37e89286275..fe453ab7fae 100644 --- a/src/kernels/kernelFinder.web.ts +++ b/src/kernels/kernelFinder.web.ts @@ -3,8 +3,7 @@ import { injectable, inject, named } from 'inversify'; import { Memento } from 'vscode'; import { GLOBAL_MEMENTO, IMemento } from '../platform/common/types'; -import { ServerConnectionType } from './jupyter/launcher/serverConnectionType'; -import { IJupyterRemoteCachedKernelValidator, IJupyterServerUriStorage } from './jupyter/types'; +import { IJupyterRemoteCachedKernelValidator, IJupyterServerUriStorage, IServerConnectionType } from './jupyter/types'; import { BaseKernelFinder } from './kernelFinder.base'; import { PreferredRemoteKernelIdProvider } from './jupyter/preferredRemoteKernelIdProvider'; import { IRemoteKernelFinder } from './raw/types'; @@ -18,7 +17,7 @@ export class KernelFinder extends BaseKernelFinder { @inject(INotebookProvider) notebookProvider: INotebookProvider, @inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento, @inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage, - @inject(ServerConnectionType) serverConnectionType: ServerConnectionType, + @inject(IServerConnectionType) serverConnectionType: IServerConnectionType, @inject(IJupyterRemoteCachedKernelValidator) protected readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator ) { diff --git a/src/kernels/serviceRegistry.node.ts b/src/kernels/serviceRegistry.node.ts index 7adc7d8f9c5..48923bd4037 100644 --- a/src/kernels/serviceRegistry.node.ts +++ b/src/kernels/serviceRegistry.node.ts @@ -38,11 +38,11 @@ import { registerTypes as registerWidgetTypes } from './ipywidgets/serviceRegist import { registerTypes as registerJupyterTypes } from './jupyter/serviceRegistry.node'; import { KernelProvider } from './kernelProvider.node'; import { KernelFinder } from './kernelFinder.node'; -import { ServerConnectionType } from './jupyter/launcher/serverConnectionType'; import { CellOutputDisplayIdTracker } from './execution/cellDisplayIdTracker'; import { DebugLocationTrackerFactory } from './debugger/debugLocationTrackerFactory'; import { Activation } from './activation.node'; import { PortAttributesProviders } from './port/portAttributeProvider.node'; +import { IServerConnectionType } from './jupyter/types'; export function registerTypes(serviceManager: IServiceManager, isDevMode: boolean) { serviceManager.addSingleton(IExtensionSingleActivationService, Activation); @@ -123,7 +123,7 @@ export function registerTypes(serviceManager: IServiceManager, isDevMode: boolea // This will ensure all subsequent telemetry will get the context of whether it is a custom/native/old notebook editor. // This is temporary, and once we ship native editor this needs to be removed. setSharedProperty('ds_notebookeditor', 'native'); - const isLocalConnection = serviceManager.get(ServerConnectionType).isLocalLaunch; + const isLocalConnection = serviceManager.get(IServerConnectionType).isLocalLaunch; setSharedProperty('localOrRemoteConnection', isLocalConnection ? 'local' : 'remote'); const isPythonExtensionInstalled = serviceManager.get(IPythonExtensionChecker); setSharedProperty( diff --git a/src/kernels/types.ts b/src/kernels/types.ts index 3c3e773ffd1..31f54ea1168 100644 --- a/src/kernels/types.ts +++ b/src/kernels/types.ts @@ -123,6 +123,14 @@ export function isLocalConnection( ); } +export function isRemoteConnection( + kernelConnection: KernelConnectionMetadata +): kernelConnection is RemoteKernelConnectionMetadata { + return ( + kernelConnection.kind === 'startUsingRemoteKernelSpec' || kernelConnection.kind === 'connectToLiveRemoteKernel' + ); +} + export interface IKernel extends IAsyncDisposable { /** * Total execution count on this kernel diff --git a/src/notebooks/controllers/installPythonControllerCommands.ts b/src/notebooks/controllers/commands/installPythonControllerCommands.ts similarity index 91% rename from src/notebooks/controllers/installPythonControllerCommands.ts rename to src/notebooks/controllers/commands/installPythonControllerCommands.ts index d94a1426ad1..9e3a38cca89 100644 --- a/src/notebooks/controllers/installPythonControllerCommands.ts +++ b/src/notebooks/controllers/commands/installPythonControllerCommands.ts @@ -10,28 +10,28 @@ import { notebooks, window } from 'vscode'; -import { IDataScienceErrorHandler } from '../../kernels/errors/types'; -import { isPythonKernelConnection } from '../../kernels/helpers'; -import { IExtensionSingleActivationService } from '../../platform/activation/types'; -import { IPythonApiProvider, IPythonExtensionChecker } from '../../platform/api/types'; -import { IApplicationShell, ICommandManager } from '../../platform/common/application/types'; +import { IDataScienceErrorHandler } from '../../../kernels/errors/types'; +import { isPythonKernelConnection } from '../../../kernels/helpers'; +import { IExtensionSingleActivationService } from '../../../platform/activation/types'; +import { IPythonApiProvider, IPythonExtensionChecker } from '../../../platform/api/types'; +import { IApplicationShell, ICommandManager } from '../../../platform/common/application/types'; import { Commands, JupyterNotebookView, PythonExtension, PYTHON_LANGUAGE, Telemetry -} from '../../platform/common/constants'; -import { ContextKey } from '../../platform/common/contextKey'; -import { IDisposableRegistry, IsWebExtension } from '../../platform/common/types'; -import { sleep } from '../../platform/common/utils/async'; -import { Common, DataScience } from '../../platform/common/utils/localize'; -import { noop } from '../../platform/common/utils/misc'; -import { traceError, traceInfo } from '../../platform/logging'; -import { ProgressReporter } from '../../platform/progress/progressReporter'; -import { sendTelemetryEvent } from '../../telemetry'; -import { getLanguageOfNotebookDocument } from '../languages/helpers'; -import { IControllerLoader, IControllerRegistration } from './types'; +} from '../../../platform/common/constants'; +import { ContextKey } from '../../../platform/common/contextKey'; +import { IDisposableRegistry, IsWebExtension } from '../../../platform/common/types'; +import { sleep } from '../../../platform/common/utils/async'; +import { Common, DataScience } from '../../../platform/common/utils/localize'; +import { noop } from '../../../platform/common/utils/misc'; +import { traceError, traceInfo } from '../../../platform/logging'; +import { ProgressReporter } from '../../../platform/progress/progressReporter'; +import { sendTelemetryEvent } from '../../../telemetry'; +import { getLanguageOfNotebookDocument } from '../../languages/helpers'; +import { IControllerLoader, IControllerRegistration } from '../types'; // This service owns the commands that show up in the kernel picker to allow for either installing // the Python Extension or installing Python diff --git a/src/notebooks/controllers/commands/serverConnectionControllerCommands.ts b/src/notebooks/controllers/commands/serverConnectionControllerCommands.ts new file mode 100644 index 00000000000..55ba15b1ae5 --- /dev/null +++ b/src/notebooks/controllers/commands/serverConnectionControllerCommands.ts @@ -0,0 +1,307 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; +import { inject, injectable } from 'inversify'; +import { IExtensionSingleActivationService } from '../../../platform/activation/types'; +import { ICommandManager, IVSCodeNotebook } from '../../../platform/common/application/types'; +import { + Commands, + InteractiveWindowView, + JupyterNotebookView, + JVSC_EXTENSION_ID +} from '../../../platform/common/constants'; +import { ContextKey } from '../../../platform/common/contextKey'; +import { IDisposableRegistry, IsWebExtension } from '../../../platform/common/types'; +import { JupyterServerSelector } from '../../../kernels/jupyter/serverSelector'; +import { createDeferred } from '../../../platform/common/utils/async'; +import { IControllerLoader, IControllerRegistration, IVSCodeNotebookController } from '../types'; +import { + IMultiStepInput, + IMultiStepInputFactory, + InputFlowAction, + InputStep +} from '../../../platform/common/utils/multiStepInput'; +import { EventEmitter, QuickPickItem, QuickPickItemKind } from 'vscode'; +import { noop } from '../../../platform/common/utils/misc'; +import { isLocalConnection } from '../../../kernels/types'; +import { IJupyterServerUriStorage, IServerConnectionType } from '../../../kernels/jupyter/types'; +import { DataScience } from '../../../platform/common/utils/localize'; + +function groupBy(data: ReadonlyArray, compare: (a: T, b: T) => number): T[][] { + const result: T[][] = []; + let currentGroup: T[] | undefined = undefined; + for (const element of data.slice(0).sort(compare)) { + if (!currentGroup || compare(currentGroup[0], element) !== 0) { + currentGroup = [element]; + result.push(currentGroup); + } else { + currentGroup.push(element); + } + } + return result; +} + +function compareIgnoreCase(a: string, b: string) { + return a.localeCompare(b, undefined, { sensitivity: 'accent' }); +} + +interface ControllerQuickPick extends QuickPickItem { + controller: IVSCodeNotebookController; +} + +// This service owns the commands that show up in the kernel picker to allow for switching +// between local and remote kernels +@injectable() +export class ServerConnectionControllerCommands implements IExtensionSingleActivationService { + private showingRemoteNotWebContext: ContextKey; + private showingLocalOrWebEmptyContext: ContextKey; + private showingRemoteContext: ContextKey; + constructor( + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(ICommandManager) private readonly commandManager: ICommandManager, + @inject(IMultiStepInputFactory) private readonly multiStepFactory: IMultiStepInputFactory, + @inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType, + @inject(IsWebExtension) private readonly isWeb: boolean, + @inject(JupyterServerSelector) private readonly serverSelector: JupyterServerSelector, + @inject(IControllerLoader) private readonly controllerLoader: IControllerLoader, + @inject(IControllerRegistration) private readonly controllerRegistration: IControllerRegistration, + @inject(IVSCodeNotebook) private readonly notebooks: IVSCodeNotebook, + @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage + ) { + // Context keys to control when these commands are shown + this.showingRemoteNotWebContext = new ContextKey('jupyter.showingRemoteNotWeb', this.commandManager); + this.showingLocalOrWebEmptyContext = new ContextKey('jupyter.showingLocalOrWebEmpty', this.commandManager); + this.showingRemoteContext = new ContextKey('jupyter.showingRemoteKernels', this.commandManager); + } + public async activate(): Promise { + this.disposables.push( + this.commandManager.registerCommand(Commands.SwitchToLocalKernels, this.switchToLocalKernels, this) + ); + this.disposables.push( + this.commandManager.registerCommand(Commands.SwitchToRemoteKernels, this.switchToRemoteKernels, this) + ); + this.disposables.push( + this.commandManager.registerCommand(Commands.SwitchToAnotherRemoteKernels, this.switchToRemoteKernels, this) + ); + this.disposables.push(this.serverConnectionType.onDidChange(this.onConnectionTypeChanged, this)); + this.onConnectionTypeChanged().ignoreErrors; + } + + private async onConnectionTypeChanged() { + const isLocal = this.serverConnectionType.isLocalLaunch; + await (this.isWeb ? this.controllerLoader.loaded : Promise.resolve(true)); + + this.showingLocalOrWebEmptyContext + .set(isLocal || (this.isWeb && this.controllerRegistration.values.length === 0)) + .ignoreErrors(); + this.showingRemoteNotWebContext.set(!isLocal && !this.isWeb).ignoreErrors(); + this.showingRemoteContext.set(!isLocal).ignoreErrors(); + } + + private async showVsCodeKernelPicker() { + const activeEditor = this.notebooks.activeNotebookEditor; + if (activeEditor) { + this.commandManager + .executeCommand('notebook.selectKernel', { notebookEditor: activeEditor }) + .then(noop, noop); + } + } + + private async switchToRemoteKernels() { + const activeNotebookType = this.notebooks.activeNotebookEditor ? JupyterNotebookView : InteractiveWindowView; + const startingLocal = this.serverConnectionType.isLocalLaunch; + const multiStep = this.multiStepFactory.create<{}>(); + return multiStep.run( + this.startSwitchRun.bind(this, this.startSwitchingToRemote.bind(this, activeNotebookType, startingLocal)), + {} + ); + } + + private async startSwitchRun(next: InputStep<{}>, _input: IMultiStepInput<{}>): Promise | void> { + // This is a middle man just to create the back button on the first step + return next; + } + + private async startSwitchingToRemote( + activeNotebookType: typeof JupyterNotebookView | typeof InteractiveWindowView, + startedLocal: boolean, + input: IMultiStepInput<{}> + ): Promise | void> { + try { + await this.serverSelector.selectJupyterURI('nonUser', input); + return this.showRemoteKernelPicker(activeNotebookType, startedLocal, input); + } catch (e) { + if (e === InputFlowAction.back) { + return this.showVsCodeKernelPicker(); + } + } + } + + private async showRemoteKernelPicker( + activeNotebookType: typeof JupyterNotebookView | typeof InteractiveWindowView, + startedLocal: boolean, + input: IMultiStepInput<{}> + ): Promise | void> { + try { + await this.showKernelPicker( + DataScience.pickRemoteKernelTitle(), + DataScience.pickRemoteKernelPlaceholder(), + false, + activeNotebookType, + input + ); + } catch (e) { + // They backed out. Put back to local + if (startedLocal) { + await this.serverSelector.setJupyterURIToLocal(); + } + throw e; + } + } + + private async switchToLocalKernels() { + const activeNotebookType = this.notebooks.activeNotebookEditor ? JupyterNotebookView : InteractiveWindowView; + const startingLocal = this.serverConnectionType.isLocalLaunch; + const startingUri = await this.serverUriStorage.getRemoteUri(); + const multiStep = this.multiStepFactory.create<{}>(); + return multiStep.run( + this.startSwitchRun.bind( + this, + this.startSwitchingToLocal.bind(this, activeNotebookType, startingLocal, startingUri) + ), + {} + ); + } + + private async startSwitchingToLocal( + activeNotebookType: typeof JupyterNotebookView | typeof InteractiveWindowView, + startedLocal: boolean, + startedUri: string | undefined, + input: IMultiStepInput<{}> + ): Promise | void> { + // Wait until we switch to local + const deferred = createDeferred(); + const disposable = this.serverConnectionType.onDidChange(() => deferred.resolve(true)); + try { + await this.serverSelector.setJupyterURIToLocal(); + await deferred.promise; + } finally { + disposable.dispose(); + } + try { + // Then bring up the quick pick for the current set of controllers. + await this.showKernelPicker( + DataScience.pickLocalKernelTitle(), + DataScience.pickLocalKernelPlaceholder(), + true, + activeNotebookType, + input + ); + } catch (e) { + // They backed out. Put back to local + if (!startedLocal && startedUri) { + await this.serverSelector.setJupyterURIToRemote(startedUri, true); + } + if (e === InputFlowAction.back) { + return this.showVsCodeKernelPicker(); + } + } + } + + private async showKernelPicker( + title: string, + placeholder: string, + local: boolean, + viewType: typeof JupyterNotebookView | typeof InteractiveWindowView, + input: IMultiStepInput<{}> + ): Promise { + // Get the current list. We will dynamically update the list as + // more and more controllers are found. + const controllers = this.controllerRegistration.values.filter( + (c) => isLocalConnection(c.connection) === local && c.viewType === viewType + ); + + // Create an event emitter for when new controllers are added + const changeEmitter = new EventEmitter<(QuickPickItem | ControllerQuickPick)[]>(); + + // Use the current controllers to generate the quick pick items + const picks: ControllerQuickPick[] = controllers.map((d) => { + return { + label: d.label, + detail: undefined, + description: d.controller.description, + controller: d + }; + }); + + // Then group them by kind + const kernelsPerCategory = groupBy(picks, (a, b) => + compareIgnoreCase(a.controller.controller.kind || 'z', b.controller.controller.kind || 'z') + ); + const kindIndexes = new Map(); + const quickPickItems: (QuickPickItem | ControllerQuickPick)[] = []; + kernelsPerCategory.forEach((items) => { + const kind = items[0].controller.controller.kind || 'Other'; + quickPickItems.push({ + kind: QuickPickItemKind.Separator, + label: kind + }); + quickPickItems.push(...items); + kindIndexes.set(kind, quickPickItems.length); + }); + + // Listen to new controllers being added + this.controllerRegistration.onCreated((e) => { + if ( + e.viewType === viewType && + quickPickItems.find((p) => (p as any).controller?.id === e.id) === undefined + ) { + // Create a pick for the new controller + const pick: ControllerQuickPick = { + label: e.label, + detail: undefined, + description: e.controller.description, + controller: e + }; + + // Stick into the list at the right place + const kind = e.controller.kind || 'Other'; + const index = kindIndexes.get(kind) || -1; + if (index < 0) { + quickPickItems.push({ + kind: QuickPickItemKind.Separator, + label: kind + }); + quickPickItems.push(pick); + kindIndexes.set(kind, quickPickItems.length); + } else { + quickPickItems.splice(index, 0, pick); + kindIndexes.set(kind, quickPickItems.length); + } + changeEmitter.fire(quickPickItems); + } + }); + + // Show quick pick with the list of controllers + const result = await input.showQuickPick({ + title: title, + items: quickPickItems, + matchOnDescription: true, + matchOnDetail: true, + startBusy: quickPickItems.length === 0, + stopBusy: this.controllerLoader.refreshed, + placeholder, + onDidChangeItems: changeEmitter.event + }); + + if (result && result.label && (result as any).controller) { + // We have selected this controller so switch to it. + const controller = (result as any).controller; + await this.commandManager.executeCommand('notebook.selectKernel', { + id: controller.id, + extension: JVSC_EXTENSION_ID + }); + } + } +} diff --git a/src/notebooks/controllers/controllerDefaultService.ts b/src/notebooks/controllers/controllerDefaultService.ts index e6f75bd3a85..121aa452a18 100644 --- a/src/notebooks/controllers/controllerDefaultService.ts +++ b/src/notebooks/controllers/controllerDefaultService.ts @@ -1,13 +1,14 @@ import { inject, injectable } from 'inversify'; import { NotebookDocument } from 'vscode'; import { isPythonNotebook } from '../../kernels/helpers'; -import { ServerConnectionType } from '../../kernels/jupyter/launcher/serverConnectionType'; +import { IServerConnectionType } from '../../kernels/jupyter/types'; import { IVSCodeNotebook } from '../../platform/common/application/types'; import { InteractiveWindowView, JupyterNotebookView, PYTHON_LANGUAGE } from '../../platform/common/constants'; import { IDisposableRegistry, IsWebExtension, Resource } from '../../platform/common/types'; import { getNotebookMetadata } from '../../platform/common/utils'; import { IInterpreterService } from '../../platform/interpreter/contracts'; import { traceInfoIfCI, traceVerbose, traceDecoratorVerbose, traceError } from '../../platform/logging'; +import { isEqual } from '../../platform/vscode-path/resources'; import { createActiveInterpreterController } from './helpers'; import { IControllerDefaultService, @@ -27,7 +28,7 @@ export class ControllerDefaultService implements IControllerDefaultService { @inject(IInterpreterService) private readonly interpreters: IInterpreterService, @inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook, @inject(IDisposableRegistry) readonly disposables: IDisposableRegistry, - @inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType, + @inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType, @inject(IsWebExtension) private readonly isWeb: boolean ) {} public async computeDefaultController( @@ -41,7 +42,7 @@ export class ControllerDefaultService implements IControllerDefaultService { traceInfoIfCI('CreateDefaultRemoteController'); const notebook = viewType === JupyterNotebookView - ? this.notebook.notebookDocuments.find((item) => item.notebookType === viewType) + ? this.notebook.notebookDocuments.find((item) => isEqual(item.uri, resource, true)) : undefined; const controller = await this.createDefaultRemoteController(viewType, notebook); // If we're running on web, there is no active interpreter to fall back to diff --git a/src/notebooks/controllers/controllerLoader.ts b/src/notebooks/controllers/controllerLoader.ts index 0e517848f33..77d6033adde 100644 --- a/src/notebooks/controllers/controllerLoader.ts +++ b/src/notebooks/controllers/controllerLoader.ts @@ -5,9 +5,8 @@ import { inject, injectable } from 'inversify'; import * as vscode from 'vscode'; import { isPythonNotebook } from '../../kernels/helpers'; import { computeServerId } from '../../kernels/jupyter/jupyterUtils'; -import { ServerConnectionType } from '../../kernels/jupyter/launcher/serverConnectionType'; import { IJupyterServerUriStorage } from '../../kernels/jupyter/types'; -import { IKernelFinder, IKernelProvider, isLocalConnection, KernelConnectionMetadata } from '../../kernels/types'; +import { IKernelFinder, IKernelProvider, KernelConnectionMetadata } from '../../kernels/types'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; import { IPythonExtensionChecker } from '../../platform/api/types'; import { IVSCodeNotebook } from '../../platform/common/application/types'; @@ -32,13 +31,11 @@ const REMOTE_KERNEL_REFRESH_INTERVAL = 2_000; */ @injectable() export class ControllerLoader implements IControllerLoader, IExtensionSyncActivationService { - private get isLocalLaunch(): boolean { - return this.serverConnectionType.isLocalLaunch; - } private wasPythonInstalledWhenFetchingControllers?: boolean; private refreshedEmitter = new vscode.EventEmitter(); // Promise to resolve when we have loaded our controllers private controllersPromise: Promise; + private loadCancellationToken: vscode.CancellationTokenSource | undefined; constructor( @inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @@ -48,7 +45,6 @@ export class ControllerLoader implements IControllerLoader, IExtensionSyncActiva @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker, @inject(IInterpreterService) private readonly interpreters: IInterpreterService, @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage, - @inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType, @inject(IControllerRegistration) private readonly registration: IControllerRegistration ) { this.loadControllers(true).ignoreErrors(); @@ -119,9 +115,13 @@ export class ControllerLoader implements IControllerLoader, IExtensionSyncActiva public loadControllers(refresh?: boolean | undefined): Promise { if (!this.controllersPromise || refresh) { const stopWatch = new StopWatch(); - const cancelToken = new vscode.CancellationTokenSource(); + // Cancel previous load + if (this.loadCancellationToken) { + this.loadCancellationToken.cancel(); + } + this.loadCancellationToken = new vscode.CancellationTokenSource(); this.wasPythonInstalledWhenFetchingControllers = this.extensionChecker.isPythonExtensionInstalled; - this.controllersPromise = this.loadControllersImpl(cancelToken.token) + this.controllersPromise = this.loadControllersImpl(this.loadCancellationToken.token) .catch((e) => { traceError('Error loading notebook controllers', e); if (!isCancellationError(e, true)) { @@ -172,9 +172,6 @@ export class ControllerLoader implements IControllerLoader, IExtensionSyncActiva } private async onDidChangeExtensions() { - if (!this.isLocalLaunch || !this.controllersPromise) { - return; - } // If we just installed the Python extension and we fetched the controllers, then fetch it again. if (!this.wasPythonInstalledWhenFetchingControllers && this.extensionChecker.isPythonExtensionInstalled) { await this.loadControllers(true); @@ -183,10 +180,6 @@ export class ControllerLoader implements IControllerLoader, IExtensionSyncActiva private async loadControllersImpl(cancelToken: vscode.CancellationToken) { let cachedConnections = await this.listKernels(cancelToken, 'useCache'); - // Remove all remove kernels if we're no longer interested in them. - if (this.isLocalLaunch) { - cachedConnections = cachedConnections.filter((connection) => isLocalConnection(connection)); - } const nonCachedConnectionsPromise = this.listKernels(cancelToken, 'ignoreCache'); traceVerbose(`Found ${cachedConnections.length} cached controllers`); @@ -208,12 +201,7 @@ export class ControllerLoader implements IControllerLoader, IExtensionSyncActiva // Never remove remote kernels that don't exist. // Always leave them there for user to select, and if the connection is not available/not valid, // then notify the user and remove them. - // Unless the user switches to using local kernels (i.e. doesn't have a remote kernel setup). - if ( - connectionIsNoLongerValid && - controller.connection.kind === 'connectToLiveRemoteKernel' && - !this.isLocalLaunch - ) { + if (connectionIsNoLongerValid && controller.connection.kind === 'connectToLiveRemoteKernel') { return true; } return connectionIsNoLongerValid; diff --git a/src/notebooks/controllers/controllerPreferredService.ts b/src/notebooks/controllers/controllerPreferredService.ts index 457e1dab5b6..7dacd449d68 100644 --- a/src/notebooks/controllers/controllerPreferredService.ts +++ b/src/notebooks/controllers/controllerPreferredService.ts @@ -16,7 +16,7 @@ import { getLanguageInNotebookMetadata, isPythonNotebook } from '../../kernels/helpers'; -import { ServerConnectionType } from '../../kernels/jupyter/launcher/serverConnectionType'; +import { IServerConnectionType } from '../../kernels/jupyter/types'; import { trackKernelResourceInformation } from '../../kernels/telemetry/helper'; import { IKernelFinder, KernelConnectionMetadata } from '../../kernels/types'; import { IExtensionSingleActivationService } from '../../platform/activation/types'; @@ -65,7 +65,7 @@ export class ControllerPreferredService implements IControllerPreferredService, @inject(IDisposableRegistry) readonly disposables: IDisposableRegistry, @inject(IKernelFinder) private readonly kernelFinder: IKernelFinder, @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker, - @inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType + @inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType ) {} public async activate() { // Sign up for document either opening or closing diff --git a/src/notebooks/controllers/controllerRegistration.ts b/src/notebooks/controllers/controllerRegistration.ts index 0692c989fff..e1c3c329e36 100644 --- a/src/notebooks/controllers/controllerRegistration.ts +++ b/src/notebooks/controllers/controllerRegistration.ts @@ -5,7 +5,8 @@ import { inject, injectable } from 'inversify'; import { Event, EventEmitter } from 'vscode'; import { getDisplayNameOrNameOfKernelConnection } from '../../kernels/helpers'; import { ILocalResourceUriConverter } from '../../kernels/ipywidgets/types'; -import { IKernelProvider, KernelConnectionMetadata } from '../../kernels/types'; +import { IJupyterServerUriStorage, IServerConnectionType } from '../../kernels/jupyter/types'; +import { IKernelProvider, isLocalConnection, isRemoteConnection, KernelConnectionMetadata } from '../../kernels/types'; import { IPythonExtensionChecker } from '../../platform/api/types'; import { IVSCodeNotebook, @@ -35,9 +36,12 @@ import { VSCodeNotebookController } from './vscodeNotebookController'; */ @injectable() export class ControllerRegistration implements IControllerRegistration { + private get isLocalLaunch(): boolean { + return this.serverConnectionType.isLocalLaunch; + } private registeredControllers = new Map(); private creationEmitter = new EventEmitter(); - private registeredConnections = new Map(); + private registeredMetadatas = new Map(); public get onCreated(): Event { return this.creationEmitter.event; @@ -45,8 +49,8 @@ export class ControllerRegistration implements IControllerRegistration { public get values(): IVSCodeNotebookController[] { return [...this.registeredControllers.values()]; } - private get connections(): KernelConnectionMetadata[] { - return [...this.registeredConnections.values()]; + private get metadatas(): KernelConnectionMetadata[] { + return [...this.registeredMetadatas.values()]; } constructor( @inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook, @@ -63,9 +67,13 @@ export class ControllerRegistration implements IControllerRegistration { @inject(IApplicationShell) private readonly appShell: IApplicationShell, @inject(IBrowserService) private readonly browser: IBrowserService, @inject(IServiceContainer) private readonly serviceContainer: IServiceContainer, - @inject(ILocalResourceUriConverter) private readonly resourceConverter: ILocalResourceUriConverter + @inject(ILocalResourceUriConverter) private readonly resourceConverter: ILocalResourceUriConverter, + @inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType, + @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage ) { - this.kernelFilter.onDidChange(this.onDidChangeKernelFilter, this, this.disposables); + this.kernelFilter.onDidChange(this.onDidChangeFilter, this, this.disposables); + this.serverConnectionType.onDidChange(this.onDidChangeFilter, this, this.disposables); + this.serverUriStorage.onDidChangeUri(this.onDidChangeUri, this, this.disposables); } add( metadata: KernelConnectionMetadata, @@ -76,12 +84,15 @@ export class ControllerRegistration implements IControllerRegistration { // Create notebook selector types .map((t) => { - return [this.getControllerId(metadata, t), t]; - }) - .filter(([id]) => { + const id = this.getControllerId(metadata, t); + // Update our list kernel connections. - this.registeredConnections.set(id, metadata); + this.registeredMetadatas.set(id, metadata); + // Return the id and the metadata for use below + return [id, t]; + }) + .filter(([id]) => { // See if we already created this controller or not const controller = this.registeredControllers.get(id); if (controller) { @@ -93,7 +104,7 @@ export class ControllerRegistration implements IControllerRegistration { // Add to results so that callers can find results.push(controller); return false; - } else if (this.kernelFilter.isKernelHidden(metadata)) { + } else if (this.isFiltered(metadata)) { // Filter out those in our kernel filter return false; } @@ -123,11 +134,19 @@ export class ControllerRegistration implements IControllerRegistration { controller.onDidDispose( () => { this.registeredControllers.delete(controller.id); - this.registeredConnections.delete(controller.id); + // Note to self, registered metadatas survive disposal. + // This is so we don't have to recompute them when we switch back + // and forth between local and remote }, this, this.disposables ); + controller.onNotebookControllerSelectionChanged((e) => { + if (!e.selected && this.isFiltered(controller.connection)) { + // This item was selected but is no longer allowed in the kernel list. Remove it + controller.dispose(); + } + }); // We are disposing as documents are closed, but do this as well this.disposables.push(controller); this.registeredControllers.set(controller.id, controller); @@ -161,6 +180,13 @@ export class ControllerRegistration implements IControllerRegistration { return this.registeredControllers.get(id); } + private isFiltered(metadata: KernelConnectionMetadata): boolean { + const userFiltered = this.kernelFilter.isKernelHidden(metadata); + const connectionTypeFiltered = isLocalConnection(metadata) !== this.isLocalLaunch; + const urlFiltered = isRemoteConnection(metadata) && this.serverUriStorage.currentServerId !== metadata.serverId; + return userFiltered || connectionTypeFiltered || urlFiltered; + } + private getControllerId( metadata: KernelConnectionMetadata, viewType: typeof JupyterNotebookView | typeof InteractiveWindowView @@ -172,12 +198,27 @@ export class ControllerRegistration implements IControllerRegistration { return this.notebook.notebookDocuments.some((doc) => controller.isAssociatedWithDocument(doc)); } - private onDidChangeKernelFilter() { - // Filter the connections. - const connections = this.connections.filter((item) => !this.kernelFilter.isKernelHidden(item)); + private onDidChangeUri() { + // Our list of metadata could be out of date. Remove old ones that don't match the uri + if (this.serverUriStorage.currentServerId) { + [...this.registeredMetadatas.keys()].forEach((k) => { + const m = this.registeredMetadatas.get(k); + if (m && isRemoteConnection(m) && this.serverUriStorage.currentServerId !== m.serverId) { + this.registeredMetadatas.delete(k); + } + }); + } + + // Update the list of controllers + this.onDidChangeFilter(); + } + + private onDidChangeFilter() { + // Give our list of metadata should be up to date, just remove the filtered ones + const metadatas = this.metadatas.filter((item) => !this.isFiltered(item)); // Try to re-create the missing controllers. - connections.forEach((c) => this.add(c, [JupyterNotebookView, InteractiveWindowView])); + metadatas.forEach((c) => this.add(c, [JupyterNotebookView, InteractiveWindowView])); // Go through all controllers that have been created and hide them. // Unless they are attached to an existing document. @@ -185,7 +226,7 @@ export class ControllerRegistration implements IControllerRegistration { // TODO: Don't hide controllers that are already associated with a notebook. // If we have a notebook opened and its using a kernel. // Else we end up killing the execution as well. - if (this.kernelFilter.isKernelHidden(item.connection) && !this.isControllerAttachedToADocument(item)) { + if (this.isFiltered(item.connection) && !this.isControllerAttachedToADocument(item)) { item.dispose(); } }); diff --git a/src/notebooks/controllers/remoteSwitcher.ts b/src/notebooks/controllers/remoteSwitcher.ts index f3feaa1af62..cf3f2906c25 100644 --- a/src/notebooks/controllers/remoteSwitcher.ts +++ b/src/notebooks/controllers/remoteSwitcher.ts @@ -53,7 +53,7 @@ export class RemoteSwitcher implements IExtensionSingleActivationService { this.updateStatusBar().catch(noop); } private async onToolBarCommand() { - await this.serverSelector.selectJupyterURI(true, 'nativeNotebookToolbar'); + await this.serverSelector.selectJupyterURI('nativeNotebookToolbar'); } private async updateStatusBar() { if (!this.notebook.activeNotebookEditor || !isJupyterNotebook(this.notebook.activeNotebookEditor.notebook)) { diff --git a/src/notebooks/controllers/types.ts b/src/notebooks/controllers/types.ts index 325fe37a703..a584852c518 100644 --- a/src/notebooks/controllers/types.ts +++ b/src/notebooks/controllers/types.ts @@ -14,6 +14,7 @@ export interface IVSCodeNotebookController extends IDisposable { readonly controller: vscode.NotebookController; readonly id: string; readonly label: string; + readonly viewType: typeof JupyterNotebookView | typeof InteractiveWindowView; readonly onNotebookControllerSelected: vscode.Event<{ notebook: vscode.NotebookDocument; controller: IVSCodeNotebookController; diff --git a/src/notebooks/controllers/vscodeNotebookController.ts b/src/notebooks/controllers/vscodeNotebookController.ts index 3fc6b6e5322..2ddd2c79701 100644 --- a/src/notebooks/controllers/vscodeNotebookController.ts +++ b/src/notebooks/controllers/vscodeNotebookController.ts @@ -27,7 +27,7 @@ import { IDocumentManager, IApplicationShell } from '../../platform/common/application/types'; -import { InteractiveWindowView, PYTHON_LANGUAGE } from '../../platform/common/constants'; +import { InteractiveWindowView, JupyterNotebookView, PYTHON_LANGUAGE } from '../../platform/common/constants'; import { disposeAllDisposables } from '../../platform/common/helpers'; import { traceInfoIfCI, @@ -116,6 +116,10 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont return this.kernelConnection; } + get viewType() { + return this._viewType as typeof InteractiveWindowView | typeof JupyterNotebookView; + } + get onNotebookControllerSelected() { return this._onNotebookControllerSelected.event; } @@ -136,7 +140,7 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont constructor( private kernelConnection: KernelConnectionMetadata, id: string, - viewType: string, + private _viewType: string, label: string, private readonly notebookApi: IVSCodeNotebook, private readonly commandManager: ICommandManager, @@ -162,7 +166,7 @@ export class VSCodeNotebookController implements Disposable, IVSCodeNotebookCont traceVerbose(`Creating notebook controller with name ${label}`); this.controller = this.notebookApi.createNotebookController( id, - viewType, + _viewType, label, this.handleExecution.bind(this), this.getRendererScripts(), diff --git a/src/notebooks/serverSelector.ts b/src/notebooks/serverSelector.ts index 7797e340ace..e1941344410 100644 --- a/src/notebooks/serverSelector.ts +++ b/src/notebooks/serverSelector.ts @@ -34,7 +34,7 @@ export class JupyterServerSelectorCommand implements IExtensionSyncActivationSer } private async selectJupyterUri( - local: boolean = true, + _local: boolean = true, // Legacy - 3rd party extenions may use this source: Uri | SelectJupyterUriCommandSource = 'commandPalette' ): Promise { if (source instanceof Uri) { @@ -44,7 +44,7 @@ export class JupyterServerSelectorCommand implements IExtensionSyncActivationSer await this.serverSelector.setJupyterURIToRemote(source.toString(true)); } else { // Activate UI Selector - this.serverSelector.selectJupyterURI(local, source).ignoreErrors(); + this.serverSelector.selectJupyterURI(source).ignoreErrors(); } // Picking the 'preferred' kernel for remote should happen in the command handler from the diff --git a/src/notebooks/serviceRegistry.node.ts b/src/notebooks/serviceRegistry.node.ts index 07d7f42ccb5..e34e0500177 100644 --- a/src/notebooks/serviceRegistry.node.ts +++ b/src/notebooks/serviceRegistry.node.ts @@ -26,7 +26,7 @@ import { NotebookIPyWidgetCoordinator } from './controllers/notebookIPyWidgetCoo import { RemoteKernelConnectionHandler } from './controllers/remoteKernelConnectionHandler'; import { JupyterServerSelectorCommand } from './serverSelector'; import { InterpreterPackageTracker } from './telemetry/interpreterPackageTracker'; -import { InstallPythonControllerCommands } from './controllers/installPythonControllerCommands'; +import { InstallPythonControllerCommands } from './controllers/commands/installPythonControllerCommands'; import { NotebookCellLanguageService } from './languages/cellLanguageService'; import { EmptyNotebookCellLanguageService } from './languages/emptyNotebookCellLanguageService'; import { DebuggingManager } from './debugger/debuggingManager'; @@ -43,6 +43,7 @@ import { FileConverter } from './export/fileConverter.node'; import { IFileConverter, INbConvertExport, ExportFormat, IExport, IExportDialog, IExportBase } from './export/types'; import { ExportUtilBase } from './export/exportUtil'; import { registerTypes as registerControllerTypes } from './controllers/serviceRegistry.node'; +import { ServerConnectionControllerCommands } from './controllers/commands/serverConnectionControllerCommands'; export function registerTypes(serviceManager: IServiceManager) { registerControllerTypes(serviceManager); @@ -100,6 +101,11 @@ export function registerTypes(serviceManager: IServiceManager) { IExtensionSingleActivationService, InstallPythonControllerCommands ); + serviceManager.addSingleton( + IExtensionSingleActivationService, + ServerConnectionControllerCommands + ); + serviceManager.addSingleton(NotebookCellLanguageService, NotebookCellLanguageService); serviceManager.addBinding(NotebookCellLanguageService, IExtensionSingleActivationService); serviceManager.addSingleton( diff --git a/src/notebooks/serviceRegistry.web.ts b/src/notebooks/serviceRegistry.web.ts index 9a5faad45af..a43d22207f3 100644 --- a/src/notebooks/serviceRegistry.web.ts +++ b/src/notebooks/serviceRegistry.web.ts @@ -37,6 +37,7 @@ import { ExportToHTML } from './export/exportToHTML'; import { ExportToPDF } from './export/exportToPDF'; import { ExportToPython } from './export/exportToPython'; import { registerTypes as registerControllerTypes } from './controllers/serviceRegistry.web'; +import { ServerConnectionControllerCommands } from './controllers/commands/serverConnectionControllerCommands'; export function registerTypes(serviceManager: IServiceManager) { registerControllerTypes(serviceManager); @@ -100,4 +101,8 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(INbConvertExport, ExportToPDF, ExportFormat.pdf); serviceManager.addSingleton(INbConvertExport, ExportToPython, ExportFormat.python); serviceManager.addSingleton(ExportUtilBase, ExportUtilBase); + serviceManager.addSingleton( + IExtensionSingleActivationService, + ServerConnectionControllerCommands + ); } diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index 27beba97d0a..6be35893137 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -278,6 +278,9 @@ export namespace Commands { export const ReplayPylanceLogStep = 'jupyter.replayPylanceLogStep'; export const InstallPythonExtensionViaKernelPicker = 'jupyter.installPythonExtensionViaKernelPicker'; export const InstallPythonViaKernelPicker = 'jupyter.installPythonViaKernelPicker'; + export const SwitchToLocalKernels = 'jupyter.switchToLocalKernels'; + export const SwitchToRemoteKernels = 'jupyter.switchToRemoteKernels'; + export const SwitchToAnotherRemoteKernels = 'jupyter.switchToAnotherRemoteKernels'; } export namespace CodeLensCommands { diff --git a/src/platform/common/utils/localize.ts b/src/platform/common/utils/localize.ts index 8856d75b6d6..06a59bc8c93 100644 --- a/src/platform/common/utils/localize.ts +++ b/src/platform/common/utils/localize.ts @@ -600,7 +600,7 @@ export namespace DataScience { export const jupyterSelectURIPrompt = () => localize('DataScience.jupyterSelectURIPrompt', 'Enter the URI of the running Jupyter server'); export const jupyterSelectURIQuickPickTitle = () => - localize('DataScience.jupyterSelectURIQuickPickTitle', 'Pick how to connect to Jupyter'); + localize('DataScience.jupyterSelectURIQuickPickTitle', 'Enter the URI of the running Jupyter server'); export const jupyterSelectURIQuickPickPlaceholder = () => localize('DataScience.jupyterSelectURIQuickPickPlaceholder', 'Choose an option'); export const jupyterSelectURIQuickPickCurrent = () => @@ -1361,6 +1361,12 @@ export namespace DataScience { localize('DataScience.webNotSupported', `Operation not supported in web version of Jupyter Extension.`); export const validationErrorMessageForRemoteUrlProtocolNeedsToBeHttpOrHttps = () => localize('DataScience.validationErrorMessageForRemoteUrlProtocolNeedsToBeHttpOrHttps', 'Has to be http(s)'); + export const pickLocalKernelTitle = () => localize('DataScience.pickLocalKernelTitle', `Select a Local Kernel`); + export const pickLocalKernelPlaceholder = () => + localize('DataScience.pickLocalKernelPlaceholder', `type to filter`); + export const pickRemoteKernelTitle = () => localize('DataScience.pickRemoteKernelTitle', `Select a Remote Kernel`); + export const pickRemoteKernelPlaceholder = () => + localize('DataScience.pickRemoteKernelPlaceholder', `type to filter`); export const failedToInstallPythonExtension = () => localize('DataScience.failedToInstallPythonExtension', `Failed to install the Python Extension.`); } diff --git a/src/platform/common/utils/multiStepInput.ts b/src/platform/common/utils/multiStepInput.ts index 6d245ac51fc..90b8c4f9903 100644 --- a/src/platform/common/utils/multiStepInput.ts +++ b/src/platform/common/utils/multiStepInput.ts @@ -32,7 +32,6 @@ export interface IQuickPickParameters { title?: string; step?: number; totalSteps?: number; - canGoBack?: boolean; items: T[]; activeItem?: T; placeholder: string; @@ -40,6 +39,9 @@ export interface IQuickPickParameters { matchOnDescription?: boolean; matchOnDetail?: boolean; acceptFilterBoxTextAsSelection?: boolean; + startBusy?: boolean; + stopBusy?: Event; + validate?(value: string): Promise; shouldResume?(): Promise; onDidTriggerItemButton?(e: QuickPickItemButtonEvent): void; onDidChangeItems?: Event; @@ -61,7 +63,7 @@ export interface InputBoxParameters { type MultiStepInputQuickPicResponseType = T | (P extends { buttons: (infer I)[] } ? I : never); type MultiStepInputInputBoxResponseType

= string | (P extends { buttons: (infer I)[] } ? I : never); export interface IMultiStepInput { - run(start: InputStep, state: S): Promise; + run(start: InputStep, state: S): Promise; showQuickPick>({ title, step, @@ -104,6 +106,9 @@ export class MultiStepInput implements IMultiStepInput { matchOnDescription, matchOnDetail, acceptFilterBoxTextAsSelection, + startBusy, + stopBusy, + validate, onDidTriggerItemButton, onDidChangeItems }: P): Promise> { @@ -117,6 +122,18 @@ export class MultiStepInput implements IMultiStepInput { input.placeholder = placeholder; input.ignoreFocusOut = true; input.items = items; + if (stopBusy) { + input.busy = startBusy ?? false; + stopBusy( + () => { + if (input.enabled) { + input.busy = false; + } + }, + this, + disposables + ); + } if (onDidChangeItems) { input.keepScrollPosition = true; onDidChangeItems( @@ -146,7 +163,25 @@ export class MultiStepInput implements IMultiStepInput { resolve(item); } }), - input.onDidChangeSelection((selectedItems) => resolve(selectedItems[0])), + input.onDidChangeSelection(async (selectedItems) => { + const itemLabel = selectedItems.length ? selectedItems[0].label : ''; + let resolvable = itemLabel ? true : false; + if (itemLabel && validate) { + input.enabled = false; + input.busy = true; + const message = await validate(itemLabel); + if (message) { + resolvable = false; + // No validation allowed on a quick pick. Have to put up a dialog instead + await this.shell.showErrorMessage(message, { modal: true }); + } + input.enabled = true; + input.busy = false; + } + if (resolvable) { + resolve(selectedItems[0]); + } + }), input.onDidHide(() => { (async () => { reject( @@ -157,8 +192,20 @@ export class MultiStepInput implements IMultiStepInput { ); if (acceptFilterBoxTextAsSelection) { disposables.push( - input.onDidAccept(() => { - resolve(input.value); + input.onDidAccept(async () => { + if (!input.busy) { + const validationMessage = validate ? await validate(input.value) : undefined; + if (!validationMessage) { + resolve(input.value); + } else { + input.enabled = false; + input.busy = true; + // No validation allowed on a quick pick. Have to put up a dialog instead + await this.shell.showErrorMessage(validationMessage, { modal: true }); + input.enabled = true; + input.busy = false; + } + } }) ); } @@ -247,7 +294,7 @@ export class MultiStepInput implements IMultiStepInput { } } - private async stepThrough(start: InputStep, state: S) { + private async stepThrough(start: InputStep, state: S): Promise { let step: InputStep | void = start; while (step) { this.steps.push(step); @@ -268,6 +315,9 @@ export class MultiStepInput implements IMultiStepInput { } else { throw err; } + if (!step) { + return err; + } } } if (this.current) { diff --git a/src/standalone/activation/activationManager.ts b/src/standalone/activation/activationManager.ts index beb685708cd..833dacc60ad 100644 --- a/src/standalone/activation/activationManager.ts +++ b/src/standalone/activation/activationManager.ts @@ -9,6 +9,7 @@ import { IExtensionSingleActivationService, IExtensionSyncActivationService } from '../../platform/activation/types'; +import { traceError } from '../../platform/logging'; @injectable() export class ExtensionActivationManager implements IExtensionActivationManager { @@ -28,7 +29,15 @@ export class ExtensionActivationManager implements IExtensionActivationManager { this.syncActivationServices.map((item) => item.activate()); } public async activate(): Promise { - // Activate all activation services together. - await Promise.all([this.singleActivationServices.map((item) => item.activate())]); + // Activate all activation services together. Don't fail them all if one fails. + await Promise.all([ + this.singleActivationServices.map(async (item) => { + const promise = item.activate(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + if (promise && (promise as any).then) { + return promise.catch(traceError); + } + }) + ]); } } diff --git a/src/test/datascience/commands/serverSelector.unit.test.ts b/src/test/datascience/commands/serverSelector.unit.test.ts index 11eb5834295..440ce3f0519 100644 --- a/src/test/datascience/commands/serverSelector.unit.test.ts +++ b/src/test/datascience/commands/serverSelector.unit.test.ts @@ -42,7 +42,7 @@ suite('DataScience - Server Selector Command', () => { handler(); - verify(serverSelector.selectJupyterURI(true, 'commandPalette')).once(); + verify(serverSelector.selectJupyterURI('commandPalette')).once(); }); test(`Command Handler should set URI`, () => { diff --git a/src/test/datascience/debugger.vscode.test.ts b/src/test/datascience/debugger.vscode.test.ts index 5045523811a..1569123c749 100644 --- a/src/test/datascience/debugger.vscode.test.ts +++ b/src/test/datascience/debugger.vscode.test.ts @@ -118,7 +118,7 @@ suite('VSCode Notebook - Run By Line', function () { ); // Give the files a quick chance to clean up - await sleep(1000); + await sleep(3000); // Now that we have finished the temp directory should be empty assert.isDefined(folderName, 'Failed to create an ipykernel debug temp folder'); diff --git a/src/test/datascience/interactive-common/notebookProvider.unit.test.ts b/src/test/datascience/interactive-common/notebookProvider.unit.test.ts index 76e29ccb567..becc98ca14b 100644 --- a/src/test/datascience/interactive-common/notebookProvider.unit.test.ts +++ b/src/test/datascience/interactive-common/notebookProvider.unit.test.ts @@ -7,11 +7,10 @@ import { PythonExtensionChecker } from '../../../platform/api/pythonApi'; import { IJupyterKernelConnectionSession, KernelConnectionMetadata } from '../../../platform/../kernels/types'; import { NotebookProvider } from '../../../kernels/jupyter/launcher/notebookProvider'; import { DisplayOptions } from '../../../kernels/displayOptions'; -import { IJupyterNotebookProvider } from '../../../kernels/jupyter/types'; +import { IJupyterNotebookProvider, IServerConnectionType } from '../../../kernels/jupyter/types'; import { IRawNotebookProvider } from '../../../kernels/raw/types'; import { IDisposable } from '../../../platform/common/types'; import { disposeAllDisposables } from '../../../platform/common/helpers'; -import { ServerConnectionType } from '../../../kernels/jupyter/launcher/serverConnectionType'; function Uri(filename: string): vscode.Uri { return vscode.Uri.file(filename); @@ -32,7 +31,7 @@ suite('DataScience - NotebookProvider', () => { when(rawNotebookProvider.isSupported).thenReturn(false); const extensionChecker = mock(PythonExtensionChecker); when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); - const connectionType = mock(); + const connectionType = mock(); when(connectionType.isLocalLaunch).thenReturn(true); const onDidChangeEvent = new vscode.EventEmitter(); disposables.push(onDidChangeEvent); diff --git a/src/test/datascience/interactiveDebugging.vscode.common.ts b/src/test/datascience/interactiveDebugging.vscode.common.ts index 00b541694e9..201f19355fa 100644 --- a/src/test/datascience/interactiveDebugging.vscode.common.ts +++ b/src/test/datascience/interactiveDebugging.vscode.common.ts @@ -96,7 +96,9 @@ export function sharedIWDebuggerTests( await closeNotebooksAndCleanUpAfterTests(disposables); }); - test('Debug a cell from a python file', async () => { + // Flakey test: https://github.com/microsoft/vscode-jupyter/issues/10521 + + test.skip('Debug a cell from a python file', async () => { // Run a cell to get IW open const source = 'print(42)'; const { activeInteractiveWindow, untitledPythonFile } = await submitFromPythonFile( diff --git a/src/test/datascience/interactiveWindow.vscode.test.ts b/src/test/datascience/interactiveWindow.vscode.test.ts index 555817e4bf6..de7c182c49a 100644 --- a/src/test/datascience/interactiveWindow.vscode.test.ts +++ b/src/test/datascience/interactiveWindow.vscode.test.ts @@ -99,7 +99,8 @@ suite(`Interactive window Execution`, async function () { await vscode.commands.executeCommand('python.clearWorkspaceInterpreter'); } - test('Export Interactive window to Notebook', async () => { + // Flakey test: https://github.com/microsoft/vscode-jupyter/issues/10649 + test.skip('Export Interactive window to Notebook', async () => { const activeInteractiveWindow = await createStandaloneInteractiveWindow(interactiveWindowProvider); await waitForInteractiveWindow(activeInteractiveWindow); diff --git a/src/test/datascience/jupyter/serverSelector.unit.test.ts b/src/test/datascience/jupyter/serverSelector.unit.test.ts index 459fd652859..6abadf7df16 100644 --- a/src/test/datascience/jupyter/serverSelector.unit.test.ts +++ b/src/test/datascience/jupyter/serverSelector.unit.test.ts @@ -7,12 +7,10 @@ import * as sinon from 'sinon'; import * as os from 'os'; import { EventEmitter, QuickPickItem } from 'vscode'; import { ApplicationShell } from '../../../platform/common/application/applicationShell'; -import { ClipboardService } from '../../../platform/common/application/clipboard'; -import { IApplicationShell, IClipboard } from '../../../platform/common/application/types'; +import { IApplicationShell } from '../../../platform/common/application/types'; import { ConfigurationService } from '../../../platform/common/configuration/service.node'; import { DataScience } from '../../../platform/common/utils/localize'; -import { MultiStepInput, MultiStepInputFactory } from '../../../platform/common/utils/multiStepInput'; -import { MockInputBox } from '../mockInputBox'; +import { MultiStepInputFactory } from '../../../platform/common/utils/multiStepInput'; import { MockQuickPick } from '../mockQuickPick'; import { MockMemento } from '../../mocks/mementos'; import { WorkspaceService } from '../../../platform/common/application/workspace.node'; @@ -26,22 +24,19 @@ import { Settings } from '../../../platform/common/constants'; import { DataScienceErrorHandler } from '../../../kernels/errors/kernelErrorHandler'; import { IDisposable } from '../../../platform/common/types'; import { disposeAllDisposables } from '../../../platform/common/helpers'; -import { ServerConnectionType } from '../../../kernels/jupyter/launcher/serverConnectionType'; import { JupyterConnection } from '../../../kernels/jupyter/jupyterConnection'; +import { IServerConnectionType } from '../../../kernels/jupyter/types'; /* eslint-disable , @typescript-eslint/no-explicit-any */ suite('DataScience - Jupyter Server URI Selector', () => { let quickPick: MockQuickPick | undefined; - let clipboard: IClipboard; let connection: JupyterConnection; let applicationShell: IApplicationShell; const disposables: IDisposable[] = []; function createDataScienceObject( quickPickSelection: string, - inputSelection: string, hasFolders: boolean ): { selector: JupyterServerSelector; storage: JupyterServerUriStorage } { - clipboard = mock(ClipboardService); const configService = mock(ConfigurationService); applicationShell = mock(ApplicationShell); const applicationEnv = mock(ApplicationEnvironment); @@ -50,9 +45,8 @@ suite('DataScience - Jupyter Server URI Selector', () => { const crypto = mock(CryptoUtils); when(crypto.createHash(anyString(), 'string')).thenCall((a1, _a2) => a1); quickPick = new MockQuickPick(quickPickSelection); - const input = new MockInputBox(inputSelection); when(applicationShell.createQuickPick()).thenReturn(quickPick!); - when(applicationShell.createInputBox()).thenReturn(input); + when(applicationShell.showErrorMessage(anything(), anything())).thenResolve(undefined); when(applicationEnv.machineId).thenReturn(os.hostname()); const multiStepFactory = new MultiStepInputFactory(instance(applicationShell)); when(workspaceService.getWorkspaceFolderIdentifier(anything())).thenReturn('1'); @@ -61,7 +55,7 @@ suite('DataScience - Jupyter Server URI Selector', () => { connection = mock(); when(connection.createConnectionInfo(anything())).thenResolve({ displayName: '' } as any); const handler = mock(DataScienceErrorHandler); - const connectionType = mock(); + const connectionType = mock(); when(connectionType.isLocalLaunch).thenReturn(false); when(connection.validateRemoteUri(anything())).thenResolve(); const onDidChangeEvent = new EventEmitter(); @@ -74,10 +68,9 @@ suite('DataScience - Jupyter Server URI Selector', () => { encryptedStorage, instance(applicationEnv), new MockMemento(), - instance(connectionType) + false ); const selector = new JupyterServerSelector( - instance(clipboard), multiStepFactory, instance(picker), storage, @@ -96,41 +89,41 @@ suite('DataScience - Jupyter Server URI Selector', () => { }); test('Local pick server uri', async () => { - const { selector, storage } = createDataScienceObject('$(zap) Default', '', true); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('', true); + await selector.selectJupyterURI(); let value = await storage.getUri(); assert.equal(value, Settings.JupyterServerLocalLaunch, 'Default should pick local launch'); // Try a second time. - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); value = await storage.getUri(); assert.equal(value, Settings.JupyterServerLocalLaunch, 'Default should pick local launch'); // Verify active items - assert.equal(quickPick?.items.length, 2, 'Wrong number of items in the quick pick'); + assert.equal(quickPick?.items.length, 0, 'Wrong number of items in the quick pick'); }); test('Local pick server uri with no workspace', async () => { - const { selector, storage } = createDataScienceObject('$(zap) Default', '', false); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('', false); + await selector.selectJupyterURI(); let value = await storage.getUri(); assert.equal(value, Settings.JupyterServerLocalLaunch, 'Default should pick local launch'); // Try a second time. - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); value = await storage.getUri(); assert.equal(value, Settings.JupyterServerLocalLaunch, 'Default should pick local launch'); // Verify active items - assert.equal(quickPick?.items.length, 2, 'Wrong number of items in the quick pick'); + assert.equal(quickPick?.items.length, 0, 'Wrong number of items in the quick pick'); }); test('Quick pick MRU tests', async () => { - const { selector, storage } = createDataScienceObject('$(zap) Default', '', true); + const { selector, storage } = createDataScienceObject('', true); console.log('Step1'); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); // Verify initial default items - assert.equal(quickPick?.items.length, 2, 'Wrong number of items in the quick pick'); + assert.equal(quickPick?.items.length, 0, 'Wrong number of items in the quick pick'); // Add in a new server const serverA1 = { uri: 'ServerA', time: 1, date: new Date(1) }; @@ -138,29 +131,29 @@ suite('DataScience - Jupyter Server URI Selector', () => { await storage.addToUriList(serverA1.uri, serverA1.time, serverA1.uri); console.log('Step3'); - await selector.selectJupyterURI(true); - assert.equal(quickPick?.items.length, 3, 'Wrong number of items in the quick pick'); - quickPickCheck(quickPick?.items[2], serverA1); + await selector.selectJupyterURI(); + assert.equal(quickPick?.items.length, 1, 'Wrong number of items in the quick pick'); + quickPickCheck(quickPick?.items[0], serverA1); // Add in a second server, the newer server should be higher in the list due to newer time const serverB1 = { uri: 'ServerB', time: 2, date: new Date(2) }; console.log('Step4'); await storage.addToUriList(serverB1.uri, serverB1.time, serverB1.uri); console.log('Step5'); - await selector.selectJupyterURI(true); - assert.equal(quickPick?.items.length, 4, 'Wrong number of items in the quick pick'); - quickPickCheck(quickPick?.items[2], serverB1); - quickPickCheck(quickPick?.items[3], serverA1); + await selector.selectJupyterURI(); + assert.equal(quickPick?.items.length, 2, 'Wrong number of items in the quick pick'); + quickPickCheck(quickPick?.items[0], serverB1); + quickPickCheck(quickPick?.items[1], serverA1); // Reconnect to server A with a new time, it should now be higher in the list const serverA3 = { uri: 'ServerA', time: 3, date: new Date(3) }; console.log('Step6'); await storage.addToUriList(serverA3.uri, serverA3.time, serverA3.uri); console.log('Step7'); - await selector.selectJupyterURI(true); - assert.equal(quickPick?.items.length, 4, 'Wrong number of items in the quick pick'); - quickPickCheck(quickPick?.items[3], serverB1); - quickPickCheck(quickPick?.items[2], serverA1); + await selector.selectJupyterURI(); + assert.equal(quickPick?.items.length, 2, 'Wrong number of items in the quick pick'); + quickPickCheck(quickPick?.items[1], serverB1); + quickPickCheck(quickPick?.items[0], serverA1); // Verify that we stick to our settings limit for (let i = 0; i < Settings.JupyterServerUriListMax + 10; i = i + 1) { @@ -169,11 +162,10 @@ suite('DataScience - Jupyter Server URI Selector', () => { } console.log('Step9'); - await selector.selectJupyterURI(true); - // Need a plus 2 here for the two default items + await selector.selectJupyterURI(); assert.equal( quickPick?.items.length, - Settings.JupyterServerUriListMax + 2, + Settings.JupyterServerUriListMax, 'Wrong number of items in the quick pick' ); }); @@ -191,143 +183,111 @@ suite('DataScience - Jupyter Server URI Selector', () => { } test('Remote server uri', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'http://localhost:1111', true); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('http://localhost:1111', true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'http://localhost:1111', 'Already running should end up with the user inputed value'); }); test('Remote server uri no workspace', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'http://localhost:1111', false); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('http://localhost:1111', false); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'http://localhost:1111', 'Already running should end up with the user inputed value'); }); test('Remote server uri no local', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'http://localhost:1111', true); - await selector.selectJupyterURI(false); + const { selector, storage } = createDataScienceObject('http://localhost:1111', true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'http://localhost:1111', 'Already running should end up with the user inputed value'); }); test('Remote server uri (reload VSCode if there is a change in settings)', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'http://localhost:1111', true); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('http://localhost:1111', true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'http://localhost:1111', 'Already running should end up with the user inputed value'); }); test('Remote server uri (do not reload VSCode if there is no change in settings)', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'http://localhost:1111', true); + const { selector, storage } = createDataScienceObject('http://localhost:1111', true); await storage.setUri('http://localhost:1111'); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'http://localhost:1111', 'Already running should end up with the user inputed value'); }); test('Invalid server uri', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'httx://localhost:1111', true); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('httx://localhost:1111', true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.notEqual(value, 'httx://localhost:1111', 'Already running should validate'); assert.equal(value, 'local', 'Validation failed'); }); test('Server is validated', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'https://localhost:1111', true); - await selector.selectJupyterURI(true); + const { selector, storage } = createDataScienceObject('https://localhost:1111', true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'https://localhost:1111', 'Validation failed'); verify(connection.validateRemoteUri('https://localhost:1111')).atLeast(1); }); test('Remote authorization is asked when ssl cert is invalid and works', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'https://localhost:1111', true); + const { selector, storage } = createDataScienceObject('https://localhost:1111', true); when(connection.validateRemoteUri(anyString())).thenReject(new Error('reason: self signed certificate')); when( applicationShell.showErrorMessage(anything(), deepEqual({ modal: true }), anything(), anything()) ).thenCall((_m, _opt, c1, _c2) => { return Promise.resolve(c1); }); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'https://localhost:1111', 'Validation failed'); verify(connection.validateRemoteUri('https://localhost:1111')).atLeast(1); }); test('Remote authorization is asked when ssl cert has expired is invalid and works', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'https://localhost:1111', true); + const { selector, storage } = createDataScienceObject('https://localhost:1111', true); when(connection.validateRemoteUri(anyString())).thenReject(new Error('reason: certificate has expired')); when( applicationShell.showErrorMessage(anything(), deepEqual({ modal: true }), anything(), anything()) ).thenCall((_m, _opt, c1, _c2) => { return Promise.resolve(c1); }); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'https://localhost:1111', 'Validation failed'); verify(connection.validateRemoteUri('https://localhost:1111')).atLeast(1); }); test('Remote authorization is asked for usage of self signed ssl cert and skipped', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'https://localhost:1111', true); + const { selector, storage } = createDataScienceObject('https://localhost:1111', true); when(connection.validateRemoteUri(anyString())).thenReject(new Error('reason: self signed certificate')); when(applicationShell.showErrorMessage(anything(), anything(), anything())).thenCall((_m, _c1, c2) => { return Promise.resolve(c2); }); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'local', 'Should not be a remote URI'); verify(connection.validateRemoteUri('https://localhost:1111')).once(); }); test('Fails to connect to remote jupyter server, hence remote jupyter server is not used', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'https://localhost:1111', true); + const { selector, storage } = createDataScienceObject('https://localhost:1111', true); when(connection.validateRemoteUri(anyString())).thenReject(new Error('Failed to connect to remote server')); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'local', 'Should not be a remote URI'); verify(connection.validateRemoteUri('https://localhost:1111')).once(); }); test('Remote authorization is asked and skipped for a different error', async () => { - const { selector, storage } = createDataScienceObject('$(server) Existing', 'https://localhost:1111', true); + const { selector, storage } = createDataScienceObject('https://localhost:1111', true); when(connection.validateRemoteUri(anyString())).thenReject(new Error('different error')); - await selector.selectJupyterURI(true); + await selector.selectJupyterURI(); const value = await storage.getUri(); assert.equal(value, 'local', 'Should not be a remote URI'); verify(connection.validateRemoteUri('https://localhost:1111')).once(); }); - - suite('Default Uri when selecting remote uri', () => { - const defaultUri = 'https://hostname:8080/?token=849d61a414abafab97bc4aab1f3547755ddc232c2b8cb7fe'; - - async function testDefaultUri(expectedDefaultUri: string, clipboardValue?: string) { - const showInputBox = sinon.spy(MultiStepInput.prototype, 'showInputBox'); - const { selector } = createDataScienceObject('$(server) Existing', 'http://localhost:1111', true); - when(clipboard.readText()).thenResolve(clipboardValue || ''); - - await selector.selectJupyterURI(true); - - assert.equal(showInputBox.firstCall.args[0].value, expectedDefaultUri); - } - - test('Display default uri', async () => { - await testDefaultUri(defaultUri); - }); - test('Display default uri if clipboard is empty', async () => { - await testDefaultUri(defaultUri, ''); - }); - test('Display default uri if clipboard contains invalid uri, display default uri', async () => { - await testDefaultUri(defaultUri, 'Hello World!'); - }); - test('Display default uri if clipboard contains invalid file uri, display default uri', async () => { - await testDefaultUri(defaultUri, 'file://test.pdf'); - }); - test('Display default uri if clipboard contains a valid uri, display uri from clipboard', async () => { - const validUri = 'https://wow:0909/?password=1234'; - - await testDefaultUri(validUri, validUri); - }); - }); }); diff --git a/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts b/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts index 6a88fa0c895..7f9e498c492 100644 --- a/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts +++ b/src/test/datascience/kernel-launcher/localKernelFinder.unit.test.ts @@ -55,8 +55,7 @@ import { PreferredRemoteKernelIdProvider } from '../../../kernels/jupyter/prefer import { NotebookProvider } from '../../../kernels/jupyter/launcher/notebookProvider'; import { RemoteKernelFinder } from '../../../kernels/jupyter/remoteKernelFinder'; import { JupyterServerUriStorage } from '../../../kernels/jupyter/launcher/serverUriStorage'; -import { ServerConnectionType } from '../../../kernels/jupyter/launcher/serverConnectionType'; -import { IJupyterRemoteCachedKernelValidator } from '../../../kernels/jupyter/types'; +import { IJupyterRemoteCachedKernelValidator, IServerConnectionType } from '../../../kernels/jupyter/types'; [false, true].forEach((isWindows) => { suite(`Local Kernel Finder ${isWindows ? 'Windows' : 'Unix'}`, () => { @@ -238,9 +237,8 @@ import { IJupyterRemoteCachedKernelValidator } from '../../../kernels/jupyter/ty preferredRemote = mock(PreferredRemoteKernelIdProvider); const notebookProvider = mock(NotebookProvider); const serverUriStorage = mock(JupyterServerUriStorage); - const connectionType = mock(); + const connectionType = mock(); when(connectionType.isLocalLaunch).thenReturn(true); - when(connectionType.setIsLocalLaunch(anything())).thenResolve(); const onDidChangeEvent = new EventEmitter(); disposables.push(onDidChangeEvent); when(connectionType.onDidChange).thenReturn(onDidChangeEvent.event); diff --git a/src/test/datascience/kernel-launcher/remoteKernelFinder.unit.test.ts b/src/test/datascience/kernel-launcher/remoteKernelFinder.unit.test.ts index 44341e0d49a..c1f026e3a8a 100644 --- a/src/test/datascience/kernel-launcher/remoteKernelFinder.unit.test.ts +++ b/src/test/datascience/kernel-launcher/remoteKernelFinder.unit.test.ts @@ -30,7 +30,8 @@ import { import { IJupyterKernel, IJupyterRemoteCachedKernelValidator, - IJupyterSessionManager + IJupyterSessionManager, + IServerConnectionType } from '../../../kernels/jupyter/types'; import { KernelFinder } from '../../../kernels/kernelFinder.node'; import { NotebookProvider } from '../../../kernels/jupyter/launcher/notebookProvider'; @@ -40,7 +41,6 @@ import { IFileSystemNode } from '../../../platform/common/platform/types.node'; import { JupyterServerUriStorage } from '../../../kernels/jupyter/launcher/serverUriStorage'; import { FileSystem } from '../../../platform/common/platform/fileSystem.node'; import { takeTopRankKernel } from './localKernelFinder.unit.test'; -import { ServerConnectionType } from '../../../kernels/jupyter/launcher/serverConnectionType'; import { LocalKernelSpecsCacheKey, RemoteKernelSpecsCacheKey } from '../../../kernels/kernelFinder.base'; suite(`Remote Kernel Finder`, () => { @@ -150,9 +150,8 @@ suite(`Remote Kernel Finder`, () => { const serverUriStorage = mock(JupyterServerUriStorage); when(serverUriStorage.getUri()).thenResolve(connInfo.baseUrl); when(serverUriStorage.getRemoteUri()).thenResolve(connInfo.baseUrl); - const connectionType = mock(); + const connectionType = mock(); when(connectionType.isLocalLaunch).thenReturn(false); - when(connectionType.setIsLocalLaunch(anything())).thenResolve(); const onDidChangeEvent = new EventEmitter(); disposables.push(onDidChangeEvent); when(connectionType.onDidChange).thenReturn(onDidChangeEvent.event); diff --git a/src/test/datascience/mockQuickPick.ts b/src/test/datascience/mockQuickPick.ts index e2524904405..cd847eafab4 100644 --- a/src/test/datascience/mockQuickPick.ts +++ b/src/test/datascience/mockQuickPick.ts @@ -29,6 +29,7 @@ export class MockQuickPick implements QuickPick { private didHideEmitter: EventEmitter = new EventEmitter(); private _activeItems: QuickPickItem[] = []; private _pickedItem: string; + private _hidden: boolean = false; constructor(pickedItem: string) { this._pickedItem = pickedItem; } @@ -71,13 +72,22 @@ export class MockQuickPick implements QuickPick { const item = this.items.find((a) => a.label === this._pickedItem); if (item) { this.didChangeSelectedEmitter.fire([item]); + } else if (this._pickedItem) { + this.value = this._pickedItem; + this.didAcceptEmitter.fire(); + setTimeout(() => { + if (!this._hidden) { + // Validation should have failed. + this.didHideEmitter.fire(); + } + }, 1); } else { this.didHideEmitter.fire(); } }, 1); } public hide(): void { - // Do nothing. + this._hidden = true; } public dispose(): void { // Do nothing. diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index a040d52273b..5508586f493 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -33,7 +33,12 @@ import { CompletionContext, CompletionTriggerKind, CancellationTokenSource, - CompletionItem + CompletionItem, + QuickPick, + QuickPickItem, + QuickInputButton, + QuickPickItemButtonEvent, + EventEmitter } from 'vscode'; import { IApplicationShell, IVSCodeNotebook, IWorkspaceService } from '../../../platform/common/application/types'; import { JVSC_EXTENSION_ID, MARKDOWN_LANGUAGE, PYTHON_LANGUAGE } from '../../../platform/common/constants'; @@ -74,6 +79,7 @@ import { } from '../../../kernels/execution/helpers'; import { chainWithPendingUpdates } from '../../../kernels/execution/notebookUpdater'; import { openAndShowNotebook } from '../../../platform/common/utils/notebooks'; +import { IServerConnectionType } from '../../../kernels/jupyter/types'; // Running in Conda environments, things can be a little slower. export const defaultNotebookTestTimeout = 60_000; @@ -262,6 +268,7 @@ export async function createEmptyPythonNotebook( traceInfoIfCI('Creating an empty notebook'); const { serviceContainer } = await getServices(); const vscodeNotebook = serviceContainer.get(IVSCodeNotebook); + const serverConnectionType = serviceContainer.get(IServerConnectionType); // Don't use same file (due to dirty handling, we might save in dirty.) // Coz we won't save to file, hence extension will backup in dirty file and when u re-open it will open from dirty. const nbFile = await createTemporaryNotebook([], disposables, 'Python 3', rootFolder, 'emptyPython'); @@ -269,7 +276,7 @@ export async function createEmptyPythonNotebook( await openAndShowNotebook(nbFile); assert.isOk(vscodeNotebook.activeNotebookEditor, 'No active notebook'); if (!dontWaitForKernel) { - await waitForKernelToGetAutoSelected(); + await waitForKernelToGetAutoSelected(undefined, !serverConnectionType.isLocalLaunch); await verifySelectedControllerIsRemoteForRemoteTests(); } await deleteAllCellsAndWait(); @@ -524,7 +531,9 @@ export async function waitForKernelToGetAutoSelected( `Houston, we have a problem, no match. Expected language ${expectedLanguage}. Expected kind ${preferredKind}.` ); assert.fail( - `No notebook controller found for ${expectedLanguage} when useRemote is ${useRemoteKernelSpec} and preferred kind is ${preferredKind}. NotebookControllers count: ${notebookControllers.length}` + `No notebook controller found for ${expectedLanguage} when useRemote is ${useRemoteKernelSpec} and preferred kind is ${preferredKind}. NotebookControllers : ${JSON.stringify( + notebookControllers.map((c) => c.connection) + )}` ); } @@ -1106,16 +1115,114 @@ export async function hijackSavePrompt( }; } +export class MockQuickPick implements QuickPick { + value: string; + placeholder: string | undefined; + get onDidChangeValue(): Event { + return this._onDidChangeValueEmitter.event; + } + get onDidAccept(): Event { + return this._onDidAcceptEmitter.event; + } + buttons: readonly QuickInputButton[]; + get onDidTriggerButton(): Event { + return this._onDidTriggerButtonEmitter.event; + } + get onDidTriggerItemButton(): Event> { + return this._onDidTriggerItemButtonEmitter.event; + } + items: readonly QuickPickItem[]; + canSelectMany: boolean; + matchOnDescription: boolean; + matchOnDetail: boolean; + keepScrollPosition?: boolean | undefined; + activeItems: readonly QuickPickItem[]; + get onDidChangeActive(): Event { + return this._onDidChangeActiveEmitter.event; + } + selectedItems: readonly QuickPickItem[]; + get onDidChangeSelection(): Event { + return this._onDidChangeSelectionEmitter.event; + } + sortByLabel: boolean; + title: string | undefined; + step: number | undefined; + totalSteps: number | undefined; + enabled: boolean; + busy: boolean; + ignoreFocusOut: boolean; + show(): void { + // Does nothing. + } + hide(): void { + this._onDidHideEmitter.fire(); + } + get onDidHide(): Event { + return this._onDidHideEmitter.event; + } + dispose(): void { + // Do nothing + } + public selectIndex(index: number) { + this.selectedItems = [this.items[index]]; + this._onDidChangeSelectionEmitter.fire([this.items[index]]); + } + public selectLastItem() { + const index = this.items.length - 1; + this.selectIndex(index); + } + public triggerButton(button: QuickInputButton): void { + this._onDidTriggerButtonEmitter.fire(button); + } + private _onDidChangeValueEmitter = new EventEmitter(); + private _onDidAcceptEmitter = new EventEmitter(); + private _onDidTriggerButtonEmitter = new EventEmitter(); + private _onDidTriggerItemButtonEmitter = new EventEmitter>(); + private _onDidChangeActiveEmitter = new EventEmitter(); + private _onDidChangeSelectionEmitter = new EventEmitter(); + private _onDidHideEmitter = new EventEmitter(); +} + +export type QuickPickStub = { + dispose(): void; + created: Event; +}; + +export async function hijackCreateQuickPick(disposables: IDisposable[] = []): Promise { + const api = await initialize(); + const appShell = api.serviceContainer.get(IApplicationShell); + const emitter = new EventEmitter(); + + const stub = sinon.stub(appShell, 'createQuickPick').callsFake(function () { + const result = new MockQuickPick(); + emitter.fire(result); + return result; + }); + const disposable = { dispose: () => stub.restore() }; + if (disposables) { + disposables.push(disposable); + disposables.push(emitter); + } + return { + dispose: () => { + stub.restore(); + emitter.dispose(); + }, + created: emitter.event + }; +} + export async function asPromise( event: Event, predicate?: (value: T) => boolean, - timeout = env.uiKind === UIKind.Desktop ? 5000 : 15000 + timeout = env.uiKind === UIKind.Desktop ? 5000 : 15000, + prefix: string | undefined = undefined ): Promise { return new Promise((resolve, reject) => { const handle = setTimeout(() => { // eslint-disable-next-line @typescript-eslint/no-use-before-define sub.dispose(); - reject(new Error('asPromise TIMEOUT reached')); + reject(new Error(`asPromise ${prefix} TIMEOUT reached`)); }, timeout); const sub = event((e) => { if (!predicate || predicate(e)) { diff --git a/src/test/datascience/notebook/kernelSelection.vscode.test.ts b/src/test/datascience/notebook/kernelSelection.vscode.test.ts index 80e19f71dbc..1a817101ce9 100644 --- a/src/test/datascience/notebook/kernelSelection.vscode.test.ts +++ b/src/test/datascience/notebook/kernelSelection.vscode.test.ts @@ -5,13 +5,13 @@ import { assert } from 'chai'; import * as fs from 'fs-extra'; import * as path from '../../../platform/vscode-path/path'; import * as sinon from 'sinon'; -import { commands, Uri, window } from 'vscode'; +import { commands, QuickInputButtons, Uri, window } from 'vscode'; import { IPythonExtensionChecker } from '../../../platform/api/types'; import { IVSCodeNotebook } from '../../../platform/common/application/types'; import { BufferDecoder } from '../../../platform/common/process/decoder.node'; import { ProcessService } from '../../../platform/common/process/proc.node'; import { IDisposable } from '../../../platform/common/types'; -import { IKernelProvider } from '../../../platform/../kernels/types'; +import { IKernelProvider, isLocalConnection, isRemoteConnection } from '../../../platform/../kernels/types'; import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { getInterpreterHash, @@ -33,11 +33,18 @@ import { waitForOutputs, waitForTextOutput, defaultNotebookTestTimeout, - createTemporaryNotebookFromFile + createTemporaryNotebookFromFile, + hijackCreateQuickPick, + asPromise } from './helper.node'; import { getOSType, OSType } from '../../../platform/common/utils/platform'; import { getTextOutputValue } from '../../../kernels/execution/helpers'; import { noop } from '../../core'; +import { Commands } from '../../../platform/common/constants'; +import { sleep } from '../../../platform/common/utils/async'; +import { IControllerLoader, IControllerRegistration, IControllerSelection } from '../../../notebooks/controllers/types'; +import { isWeb } from '../../../platform/common/utils/misc'; +import { IJupyterServerUriStorage } from '../../../kernels/jupyter/types'; /* eslint-disable no-invalid-this, , , @typescript-eslint/no-explicit-any */ suite('DataScience - VSCode Notebook - Kernel Selection', function () { @@ -69,6 +76,11 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () { const venvNoRegSearchString = '.venvnoreg'; let activeInterpreterSearchString = ''; let vscodeNotebook: IVSCodeNotebook; + let controllerRegistration: IControllerRegistration; + let controllerLoader: IControllerLoader; + let controllerSelection: IControllerSelection; + let serverUriStorage: IJupyterServerUriStorage; + let jupyterServerUri: string | undefined; this.timeout(120_000); // Slow test, we need to uninstall/install ipykernel. /* This test requires a virtual environment to be created & registered as a kernel. @@ -91,6 +103,10 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () { const pythonChecker = api.serviceContainer.get(IPythonExtensionChecker); vscodeNotebook = api.serviceContainer.get(IVSCodeNotebook); kernelProvider = api.serviceContainer.get(IKernelProvider); + controllerRegistration = api.serviceContainer.get(IControllerRegistration); + serverUriStorage = api.serviceContainer.get(IJupyterServerUriStorage); + controllerLoader = api.serviceContainer.get(IControllerLoader); + controllerSelection = api.serviceContainer.get(IControllerSelection); if (!pythonChecker.isPythonExtensionInstalled) { return this.skip(); @@ -131,6 +147,7 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () { ]); await startJupyterServer(); + jupyterServerUri = await serverUriStorage.getRemoteUri(); sinon.restore(); }); @@ -155,6 +172,9 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () { console.log(`End test ${this.currentTest?.title}`); await closeNotebooksAndCleanUpAfterTests(disposables); console.log(`End test completed ${this.currentTest?.title}`); + if (jupyterServerUri) { + await serverUriStorage.setUriToRemote(jupyterServerUri, ''); + } }); test('Ensure we select active interpreter as kernel (when Raw Kernels)', async function () { @@ -341,4 +361,321 @@ suite('DataScience - VSCode Notebook - Kernel Selection', function () { waitForTextOutput(cell, venvNoRegSearchString, 0, false) ]); }); + + async function verifyExpectedCounts(localIsExpected: boolean) { + await controllerLoader.loaded; + const remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + const locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + + if (localIsExpected) { + assert.ok(locals.length > 1, 'Expected at least two local controller'); + assert.ok(remotes.length <= 1, 'Expected at most one remote controller'); + } else { + assert.ok(locals.length <= 1, 'Expected at most one local controller'); + assert.ok(remotes.length >= 1, 'Expected at least one remote controller'); + } + } + + test('Start local, pick remote second level and go back - locals should be shown', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await serverUriStorage.setUriToLocal(); + await controllerLoader.loaded; + await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + // We should start off with locals + assert.ok(locals.length > 0, 'No locals found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:back'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToRemoteKernels) as Promise; + + // URI quick pick should be on the screen. + let uriQuickPick = await firstPromise; + + // Trigger the selection of the first URI (should be our remote URI) + const secondPromise = asPromise(created, undefined, 5000, 'second:back'); + uriQuickPick.selectIndex(0); + + // Wait for the 'remote' list to show up + const remoteQuickPick = await secondPromise; + await controllerLoader.loaded; + + // At this point we should have remote kernels + let remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + assert.ok(remotes.length > 0, 'No remote kernels found'); + + // Trigger the back button on the remote list + const thirdPromise = asPromise(created, undefined, 5000, 'third:back'); + remoteQuickPick.triggerButton(QuickInputButtons.Back); + + // That should bring up the URI list again + uriQuickPick = await thirdPromise; + + // Trigger back on that + uriQuickPick.triggerButton(QuickInputButtons.Back); + + // Wait for the command to complete + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notOk(result, `Back button did not finish the command`); + + // Make sure our kernel list is only locals + await verifyExpectedCounts(true); + }); + + test('Start local, pick remote - remotes should be shown', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await serverUriStorage.setUriToLocal(); + await controllerLoader.loaded; + const notebook = await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + // We should start off with locals + assert.ok(locals.length > 0, 'No locals found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:back'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToRemoteKernels) as Promise; + + // URI quick pick should be on the screen. + let uriQuickPick = await firstPromise; + + // Trigger the selection of the first URI (should be our remote URI) + const secondPromise = asPromise(created, undefined, 5000, 'second:back'); + uriQuickPick.selectIndex(0); + + // Wait for the 'remote' list to show up + const remoteQuickPick = await secondPromise; + await controllerLoader.loaded; + + // At this point we should have remote kernels + let remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + assert.ok(remotes.length > 0, 'No remote kernels found'); + + // Pick a remote kernel + remoteQuickPick.selectLastItem(); + + // Wait for the command to complete + const selectionChanged = asPromise(controllerSelection.onControllerSelected); + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notOk(result, `Picking remote did not complete`); + + // Make sure our kernel list is remotes + await verifyExpectedCounts(false); + + // Make sure the selected kernel is remote + await selectionChanged; + const selected = controllerSelection.getSelected(notebook); + assert.ok(selected, 'Remote kernel was not selected'); + assert.ok(isRemoteConnection(selected!.connection), 'Selected kernel is not remote'); + }); + + test('Start local, pick remote first level and go back - locals should be shown', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await serverUriStorage.setUriToLocal(); + await controllerLoader.loaded; + await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + // We should start off with locals + assert.ok(locals.length > 0, 'No locals found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:back'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToRemoteKernels) as Promise; + + // URI quick pick should be on the screen. + let uriQuickPick = await firstPromise; + + // Trigger the back button on the uri list + uriQuickPick.triggerButton(QuickInputButtons.Back); + + // Wait for the command to complete + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notOk(result, `Back button did not finish the command`); + + // Make sure our kernel list is only locals + await verifyExpectedCounts(true); + }); + + test('Start local, pick remote and cancel - locals should be shown', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await serverUriStorage.setUriToLocal(); + await controllerLoader.loaded; + await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + // We should start off with locals + assert.ok(locals.length > 0, 'No locals found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:cancel'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToRemoteKernels) as Promise; + + // URI quick pick should be on the screen. + let uriQuickPick = await firstPromise; + + // Trigger the selection of the first URI (should be our remote URI) + const secondPromise = asPromise(created, undefined, 5000, 'second:cancel'); + uriQuickPick.selectIndex(0); + + // Wait for the 'remote' list to show up + const remoteQuickPick = await secondPromise; + await controllerLoader.loaded; + + // At this point we should have remote kernels + let remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + assert.ok(remotes.length > 0, 'No remote kernels found'); + + // Trigger a cancel + remoteQuickPick.hide(); + + // Wait for the command to complete + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notEqual(result, 5000, `Cancel button did not finish the command`); + + // Make sure our kernel list is only locals + await verifyExpectedCounts(true); + }); + test('Start remote, pick local and cancel - remotes should be shown', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await controllerLoader.loaded; + await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + // We should start off with remotes + assert.ok(remotes.length > 0, 'No remotes found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:cancel'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToLocalKernels) as Promise; + + // Locals quick pick should be on the first screen + let localsQuickPick = await firstPromise; + await controllerLoader.loaded; + + // Should have all locals at the moment + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + assert.ok(locals.length > 1, 'No local connections'); + + // Cancel locals + localsQuickPick.hide(); + + // Wait for the command to complete + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notEqual(result, 5000, `Cancel button did not finish the command`); + + // Make sure our kernel list is only remotes + await verifyExpectedCounts(false); + }); + + test('Start remote, pick local and back - remotes should be shown', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await controllerLoader.loaded; + await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + // We should start off with remotes + assert.ok(remotes.length > 0, 'No remotes found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:cancel'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToLocalKernels) as Promise; + + // Locals quick pick should be on the first screen + let localsQuickPick = await firstPromise; + await controllerLoader.loaded; + + // Should have all locals at the moment + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + assert.ok(locals.length > 0, 'No local connections'); + + // Hit back + localsQuickPick.triggerButton(QuickInputButtons.Back); + + // Wait for the command to complete + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notEqual(result, 5000, `Cancel button did not finish the command`); + + // Make sure our kernel list is only remotes + await verifyExpectedCounts(false); + }); + test('Start remote, pick local. Make sure it is picked', async function () { + if (!IS_REMOTE_NATIVE_TEST() || isWeb()) { + this.skip(); // Test only works when have a remote server mode and we can connect to locals + } + await controllerLoader.loaded; + const notebook = await createEmptyPythonNotebook(disposables); + await insertCodeCell('import sys\nsys.executable', { index: 0 }); + + let remotes = controllerRegistration.values.filter((c) => isRemoteConnection(c.connection)); + // We should start off with remotes + assert.ok(remotes.length > 0, 'No remotes found'); + + // Hijack the quick pick so we can force picking back + const { created } = await hijackCreateQuickPick(disposables); + const firstPromise = asPromise(created, undefined, 5000, 'first:cancel'); + + // Execute the command but don't wait for it + const commandPromise = commands.executeCommand(Commands.SwitchToLocalKernels) as Promise; + + // Locals quick pick should be on the first screen + let localsQuickPick = await firstPromise; + await controllerLoader.loaded; + + // Should have all locals at the moment + let locals = controllerRegistration.values.filter((c) => isLocalConnection(c.connection)); + assert.ok(locals.length > 1, 'No local connections'); + + // Pick a local connection + localsQuickPick.selectLastItem(); + + // Wait for the command to complete + const selectionChanged = asPromise(controllerSelection.onControllerSelected); + const result = await Promise.race([commandPromise, sleep(5000)]); + assert.notEqual(result, 5000, `Cancel button did not finish the command`); + + // Make sure our kernel list is only locals + await verifyExpectedCounts(true); + await selectionChanged; + const selected = controllerSelection.getSelected(notebook); + assert.ok(selected, 'Local kernel was not selected'); + assert.ok(isLocalConnection(selected!.connection), 'Selected kernel is not local'); + }); }); diff --git a/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts b/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts index 31c08f7b18f..d8c85647d46 100644 --- a/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts +++ b/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts @@ -157,20 +157,20 @@ suite('DataScience - VSCode Notebook - (Remote Execution)', function () { ); }); - test('Local and Remote kernels are listed', async function () { + test('Local and Remote kernels are not listed', async function () { await controllerLoader.loadControllers(); const controllers = controllerRegistration.values; assert.ok( controllers.some((item) => item.connection.kind === 'startUsingRemoteKernelSpec'), 'Should have at least one remote kernelspec' ); - assert.ok( + assert.notOk( controllers.some( (item) => item.connection.kind === 'startUsingLocalKernelSpec' || item.connection.kind === 'startUsingPythonInterpreter' ), - 'Should have at least one local kernel' + 'Should have no local kernels' ); }); test('Remote kernels are removed when switching to local', async function () { diff --git a/src/test/initialize.node.ts b/src/test/initialize.node.ts index e12698db78a..8c805e40181 100644 --- a/src/test/initialize.node.ts +++ b/src/test/initialize.node.ts @@ -5,9 +5,6 @@ import { IS_SMOKE_TEST } from './constants.node'; import { startJupyterServer } from './datascience/notebook/helper.node'; import { PythonExtension, setTestExecution } from '../platform/common/constants'; import { activateExtension, closeActiveWindows } from './initialize'; -import { IS_REMOTE_NATIVE_TEST } from './constants'; -import { ServerConnectionType } from '../kernels/jupyter/launcher/serverConnectionType'; -import { noop } from './core'; export * from './initialize'; export * from './constants.node'; @@ -40,9 +37,6 @@ export async function initialize(): Promise { jupyterServerStarted = true; await startJupyterServer(); } - if (IS_REMOTE_NATIVE_TEST()) { - api.serviceContainer.get(ServerConnectionType).setIsLocalLaunch(false).then(noop, noop); - } return api; } diff --git a/src/test/kernels/jupyter/jupyterConnection.unit.test.ts b/src/test/kernels/jupyter/jupyterConnection.unit.test.ts index 2ba54cf19d2..b2c2379f5d9 100644 --- a/src/test/kernels/jupyter/jupyterConnection.unit.test.ts +++ b/src/test/kernels/jupyter/jupyterConnection.unit.test.ts @@ -7,12 +7,12 @@ import { assert, use } from 'chai'; import { anything, instance, mock, verify, when } from 'ts-mockito'; import { EventEmitter } from 'vscode'; import { JupyterConnection } from '../../../kernels/jupyter/jupyterConnection'; -import { ServerConnectionType } from '../../../kernels/jupyter/launcher/serverConnectionType'; import { IJupyterServerUriStorage, IJupyterSessionManager, IJupyterSessionManagerFactory, - IJupyterUriProviderRegistration + IJupyterUriProviderRegistration, + IServerConnectionType } from '../../../kernels/jupyter/types'; import { disposeAllDisposables } from '../../../platform/common/helpers'; import { IDisposable } from '../../../platform/common/types'; @@ -22,14 +22,14 @@ suite('Jupyter Connection', async () => { let jupyterConnection: JupyterConnection; let registrationPicker: IJupyterUriProviderRegistration; let sessionManagerFactory: IJupyterSessionManagerFactory; - let serverConnectionType: ServerConnectionType; + let serverConnectionType: IServerConnectionType; let sessionManager: IJupyterSessionManager; let serverUriStorage: IJupyterServerUriStorage; const disposables: IDisposable[] = []; setup(() => { registrationPicker = mock(); sessionManagerFactory = mock(); - serverConnectionType = mock(); + serverConnectionType = mock(); sessionManager = mock(); serverUriStorage = mock(); jupyterConnection = new JupyterConnection(