Fix problems with context keys and strings for remote connections (#1…

…0768) * Fix context problems in kernel selection * Add news
microsoft · Jul 12, 2022 · 708004b · 708004b
1 parent 06d0fbd
commit 708004b
Show file tree

Hide file tree

Showing 7 changed files with 59 additions and 33 deletions.
diff --git a/TELEMETRY.md b/TELEMETRY.md
@@ -903,7 +903,7 @@ No properties for event
 
  @captureTelemetry(Telemetry.EnterJupyterURI)
  @traceDecoratorError('Failed to enter Jupyter Uri')
- public async setJupyterURIToRemote(userURI: string, ignoreValidation?: boolean): Promise<void> {
+ public async setJupyterURIToRemote(userURI: string | undefined, ignoreValidation?: boolean): Promise<void> {
  // Double check this server can be connected to. Might need a password, might need a allowUnauthorized
 ```
 
@@ -4315,13 +4315,13 @@ No description provided
 
 [src/kernels/jupyter/serverSelector.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/serverSelector.ts)
 ```typescript
- await this.serverUriStorage.setUriToRemote(userURI, connection.displayName);
+  await this.serverUriStorage.setUriToRemote(userURI, connection.displayName);
 
- // Indicate setting a jupyter URI to a remote setting. Check if an azure remote or not
- sendTelemetryEvent(Telemetry.SetJupyterURIToUserSpecified, undefined, {
- azure: userURI.toLowerCase().includes('azure')
- });
- }
+  // Indicate setting a jupyter URI to a remote setting. Check if an azure remote or not
+  sendTelemetryEvent(Telemetry.SetJupyterURIToUserSpecified, undefined, {
+  azure: userURI.toLowerCase().includes('azure')
+  });
+  } else {
 ```
 
 </details>
@@ -8894,7 +8894,7 @@ No properties for event
 [src/kernels/jupyter/serverSelector.ts](https://github.com/microsoft/vscode-jupyter/tree/main/src/kernels/jupyter/serverSelector.ts)
 ```typescript
  }
- } else {
+ } else if (userURI) {
  if (err.message.includes('Failed to fetch') && this.isWebExtension) {
  sendTelemetryEvent(Telemetry.FetchError, undefined, { currentTask: 'connecting' });
  }

diff --git a/news/2 Fixes/10672.md b/news/2 Fixes/10672.md
@@ -0,0 +1 @@
+Fix language of picking a jupyter server when in the web extension.
diff --git a/package.nls.json b/package.nls.json
@@ -780,7 +780,7 @@
  "comment": ["{Locked='python -m jupyter notebook --version'}"]
  },
  "DataScience.savePngTitle": "Save Image",
- "DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter",
+ "DataScience.jupyterSelectURIQuickPickTitle": "Enter the URI of the running Jupyter server",
  "DataScience.jupyterSelectURIQuickPickPlaceholder": "Choose an option",
  "DataScience.jupyterSelectURIQuickPickCurrent": "Current: {0}",
  "DataScience.jupyterSelectURINoneLabel": "None",

diff --git a/src/kernels/jupyter/launcher/serverUriStorage.ts b/src/kernels/jupyter/launcher/serverUriStorage.ts
@@ -25,7 +25,7 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServe
  private lastSavedList?: Promise<
  { uri: string; serverId: string; time: number; displayName?: string | undefined }[]
  >;
- private currentUriPromise: Promise<string> | undefined;
+ private currentUriPromise: Promise<string | undefined> | undefined;
  private _currentServerId: string | undefined;
  private _localOnly: boolean = false;
  private _onDidChangeUri = new EventEmitter<void>();
@@ -190,7 +190,7 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServe
  })
  );
  }
- public getUri(): Promise<string> {
+ public getUri(): Promise<string | undefined> {
  if (!this.currentUriPromise) {
  this.currentUriPromise = this.getUriInternal();
  }
@@ -226,26 +226,30 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServe
  await this.setUri(uri);
  }
 
- public async setUri(uri: string) {
+ public async setUriToNone(): Promise<void> {
+ return this.setUri(undefined);
+ }
+
+ public async setUri(uri: string | undefined) {
  // Set the URI as our current state
  this.currentUriPromise = Promise.resolve(uri);
- this._currentServerId = await computeServerId(uri);
- this._localOnly = uri === Settings.JupyterServerLocalLaunch || uri === undefined;
+ this._currentServerId = uri ? await computeServerId(uri) : undefined;
+ this._localOnly = (uri === Settings.JupyterServerLocalLaunch || uri === undefined) && !this.isWebExtension;
  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) {
+ if (!this._localOnly && uri) {
  await this.addToUriList(uri, Date.now(), uri);
 
  // Save in the storage (unique account per workspace)
  const key = this.getUriAccountKey();
  await this.encryptedStorage.store(Settings.JupyterServerRemoteLaunchService, key, uri);
  }
  }
- private async getUriInternal(): Promise<string> {
+ private async getUriInternal(): Promise<string | undefined> {
  if (this.isLocalLaunch) {
  return Settings.JupyterServerLocalLaunch;
  } else {
@@ -258,7 +262,7 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServe
  this._currentServerId = await computeServerId(storedUri);
  }
 
- return storedUri || Settings.JupyterServerLocalLaunch;
+ return storedUri;
  }
  }
 

diff --git a/src/kernels/jupyter/serverSelector.ts b/src/kernels/jupyter/serverSelector.ts
@@ -87,10 +87,10 @@ export class JupyterServerSelector {
 
  @captureTelemetry(Telemetry.EnterJupyterURI)
  @traceDecoratorError('Failed to enter Jupyter Uri')
- public async setJupyterURIToRemote(userURI: string, ignoreValidation?: boolean): Promise<void> {
+ public async setJupyterURIToRemote(userURI: string | undefined, ignoreValidation?: boolean): Promise<void> {
  // Double check this server can be connected to. Might need a password, might need a allowUnauthorized
  try {
- if (!ignoreValidation) {
+ if (!ignoreValidation && userURI) {
  await this.jupyterConnection.validateRemoteUri(userURI);
  }
  } catch (err) {
@@ -106,7 +106,7 @@ export class JupyterServerSelector {
  if (!handled) {
  return;
  }
- } else {
+ } else if (userURI) {
  if (err.message.includes('Failed to fetch') && this.isWebExtension) {
  sendTelemetryEvent(Telemetry.FetchError, undefined, { currentTask: 'connecting' });
  }
@@ -117,13 +117,17 @@ export class JupyterServerSelector {
  }
  }
 
- const connection = await this.jupyterConnection.createConnectionInfo({ uri: userURI });
- await this.serverUriStorage.setUriToRemote(userURI, connection.displayName);
+ if (userURI) {
+ const connection = await this.jupyterConnection.createConnectionInfo({ uri: userURI });
+ await this.serverUriStorage.setUriToRemote(userURI, connection.displayName);
 
- // Indicate setting a jupyter URI to a remote setting. Check if an azure remote or not
- sendTelemetryEvent(Telemetry.SetJupyterURIToUserSpecified, undefined, {
- azure: userURI.toLowerCase().includes('azure')
- });
+ // Indicate setting a jupyter URI to a remote setting. Check if an azure remote or not
+ sendTelemetryEvent(Telemetry.SetJupyterURIToUserSpecified, undefined, {
+ azure: userURI.toLowerCase().includes('azure')
+ });
+ } else {
+ await this.serverUriStorage.setUriToNone();
+ }
  }
 
  private async startSelectingURI(input: IMultiStepInput<{}>, _state: {}): Promise<InputStep<{}> | void> {

diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts
@@ -241,6 +241,7 @@ export interface IJupyterServerUriStorage {
  getRemoteUri(): Promise<string | undefined>;
  setUriToLocal(): Promise<void>;
  setUriToRemote(uri: string, displayName: string): Promise<void>;
+ setUriToNone(): Promise<void>;
 }
 
 export interface IBackupFile {

diff --git a/src/notebooks/controllers/commands/serverConnectionControllerCommands.ts b/src/notebooks/controllers/commands/serverConnectionControllerCommands.ts
@@ -84,24 +84,27 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  this.disposables.push(
  this.commandManager.registerCommand(Commands.SwitchToAnotherRemoteKernels, this.switchToRemoteKernels, this)
  );
- this.disposables.push(this.serverConnectionType.onDidChange(this.onConnectionTypeChanged, this));
- this.onConnectionTypeChanged().ignoreErrors;
+ this.disposables.push(this.serverConnectionType.onDidChange(this.updateContextKeys, this));
+ this.updateContextKeys().ignoreErrors;
  }
 
- private async onConnectionTypeChanged() {
+ private async updateContextKeys() {
  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();
+ this.showingRemoteContext.set(!isLocal && this.controllerRegistration.values.length > 0).ignoreErrors();
  }
 
  private async showVsCodeKernelPicker() {
  const activeEditor = this.notebooks.activeNotebookEditor;
  if (activeEditor) {
+ // Need to wait for controller to reupdate after
+ // switching local/remote
+ await this.controllerLoader.loaded;
  this.commandManager
  .executeCommand('notebook.selectKernel', { notebookEditor: activeEditor })
  .then(noop, noop);
@@ -111,9 +114,13 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  private async switchToRemoteKernels() {
  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.startSwitchingToRemote.bind(this, activeNotebookType, startingLocal)),
+ this.startSwitchRun.bind(
+ this,
+ this.startSwitchingToRemote.bind(this, activeNotebookType, startingLocal, startingUri)
+ ),
  {}
  );
  }
@@ -126,11 +133,12 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  private async startSwitchingToRemote(
  activeNotebookType: typeof JupyterNotebookView | typeof InteractiveWindowView,
  startedLocal: boolean,
+ startingUri: string | undefined,
  input: IMultiStepInput<{}>
  ): Promise<InputStep<{}> | void> {
  try {
  await this.serverSelector.selectJupyterURI('nonUser', input);
- return this.showRemoteKernelPicker(activeNotebookType, startedLocal, input);
+ return this.showRemoteKernelPicker(activeNotebookType, startedLocal, startingUri, input);
  } catch (e) {
  if (e === InputFlowAction.back) {
  return this.showVsCodeKernelPicker();
@@ -141,6 +149,7 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  private async showRemoteKernelPicker(
  activeNotebookType: typeof JupyterNotebookView | typeof InteractiveWindowView,
  startedLocal: boolean,
+ startedUri: string | undefined,
  input: IMultiStepInput<{}>
  ): Promise<InputStep<{}> | void> {
  try {
@@ -155,6 +164,9 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  // They backed out. Put back to local
  if (startedLocal) {
  await this.serverSelector.setJupyterURIToLocal();
+ } else {
+ // Web case is never local but we might have an empty URI
+ await this.serverSelector.setJupyterURIToRemote(startedUri);
  }
  throw e;
  }
@@ -199,7 +211,7 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  input
  );
  } catch (e) {
- // They backed out. Put back to local
+ // They backed out. Put back to remote
  if (!startedLocal && startedUri) {
  await this.serverSelector.setJupyterURIToRemote(startedUri, true);
  }
@@ -302,6 +314,10 @@ export class ServerConnectionControllerCommands implements IExtensionSingleActiv
  id: controller.id,
  extension: JVSC_EXTENSION_ID
  });
+
+ // Update our context keys as they might have changed when
+ // moving to a new kernel
+ await this.updateContextKeys();
  }
  }
 }