Remove uri from INotebookServerOptions (#9957)

src/kernels/jupyter/jupyterConnection.ts

@@ -6,10 +6,11 @@ import { IExtensionSyncActivationService } from '../../platform/activation/types
 import { Identifiers } from '../../platform/common/constants';
 import { IDisposableRegistry } from '../../platform/common/types';
 import { IJupyterConnection } from '../types';
-import { createRemoteConnectionInfo } from './jupyterUtils';
+import { computeUriHash, createRemoteConnectionInfo } from './jupyterUtils';
 import { ServerConnectionType } from './launcher/serverConnectionType';
 import {
  IJupyterServerUri,
+ IJupyterServerUriStorage,
  IJupyterSessionManager,
  IJupyterSessionManagerFactory,
  IJupyterUriProviderRegistration,
@@ -27,7 +28,8 @@ export class JupyterConnection implements IExtensionSyncActivationService {
  private readonly jupyterSessionManagerFactory: IJupyterSessionManagerFactory,
  @inject(IDisposableRegistry)
  private readonly disposables: IDisposableRegistry,
- @inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType
+ @inject(ServerConnectionType) private readonly serverConnectionType: ServerConnectionType,
+ @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage
  ) {
  disposables.push(this);
  }
@@ -48,14 +50,28 @@ export class JupyterConnection implements IExtensionSyncActivationService {
  this.pendingTimeouts.forEach((t) => clearTimeout(t as any));
  this.pendingTimeouts = [];
  }
- public async createConnectionInfo(uri: string) {
+
+ public async createConnectionInfo(serverId: string) {
+ const uri = await this.getUriFromServerId(serverId);
+ if (!uri) {
+ throw new Error('Server Not found');
+ }
+ return this.createConnectionInfoFromUri(uri);
+ }
+ public async validateRemoteUri(uri: string): Promise<void> {
+ return this.validateRemoteConnection(await this.createConnectionInfoFromUri(uri));
+ }
+
+ private async getUriFromServerId(serverId: string) {
+ // Since there's one server per session, don't use a resource to figure out these settings
+ const savedList = await this.serverUriStorage.getSavedUriList();
+ return savedList.find((item) => computeUriHash(item.uri) === serverId)?.uri;
+ }
+ private async createConnectionInfoFromUri(uri: string) {
  // Prepare our map of server URIs
  await this.updateServerUri(uri);
  return createRemoteConnectionInfo(uri, this.getServerUri.bind(this));
  }
- public async validateRemoteUri(uri: string): Promise<void> {
- return this.validateRemoteConnection(await this.createConnectionInfo(uri));
- }
 
  private async validateRemoteConnection(connection: IJupyterConnection): Promise<void> {
  let sessionManager: IJupyterSessionManager | undefined = undefined;

src/kernels/jupyter/launcher/jupyterExecution.ts

@@ -210,7 +210,7 @@ export class JupyterExecutionBase implements IJupyterExecution {
  );
  } else {
  // If we have a URI spec up a connection info for it
- return this.jupyterConnection.createConnectionInfo(options.uri);
+ return this.jupyterConnection.createConnectionInfo(options.serverId);
  }
  }
 

src/kernels/jupyter/launcher/liveshare/serverCache.ts

@@ -110,7 +110,6 @@ export class ServerCache implements IAsyncDisposable {
  }
  return {
  serverId: options.serverId,
- uri: options.uri,
  resource: options?.resource,
  ui: options.ui,
  localJupyter: false
@@ -119,7 +118,6 @@ export class ServerCache implements IAsyncDisposable {
 
  private generateKey(options: INotebookServerOptions): string {
  // combine all the values together to make a unique key
- const uri = options.localJupyter ? '' : options.uri.toString();
- return `uri=${uri};local=${options.localJupyter};`;
+ return `serverId=${options.localJupyter ? '' : options.serverId};local=${options.localJupyter};`;
  }
 }

src/kernels/jupyter/launcher/notebookServerProvider.ts

@@ -24,7 +24,6 @@ import {
 } from '../types';
 import { NotSupportedInWebError } from '../../../platform/errors/notSupportedInWebError';
 import { getFilePath } from '../../../platform/common/platform/fs-paths';
-import { computeUriHash } from '../jupyterUtils';
 
 const localCacheKey = 'LocalJupyterSererCacheKey';
 @injectable()
@@ -34,7 +33,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {
  constructor(
  @inject(IJupyterExecution) @optional() private readonly jupyterExecution: IJupyterExecution | undefined,
  @inject(IInterpreterService) private readonly interpreterService: IInterpreterService,
- @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage,
+ @inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage,
  @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry
  ) {
  serverUriStorage.onDidChangeUri(
@@ -56,7 +55,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {
  this.serverPromise.clear();
  }
  public async getOrCreateServer(options: GetServerOptions): Promise<INotebookServer> {
- const serverOptions = await this.getNotebookServerOptions(options);
+ const serverOptions = this.getNotebookServerOptions(options);
 
  // If we are just fetching or only want to create for local, see if exists
  if (options.localJupyter && this.jupyterExecution) {
@@ -102,7 +101,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {
  if (!jupyterExecution) {
  throw new NotSupportedInWebError();
  }
- const serverOptions = await this.getNotebookServerOptions(options);
+ const serverOptions = this.getNotebookServerOptions(options);
  traceInfo(`Checking for server existence.`);
 
  const disposables: IDisposable[] = [];
@@ -183,7 +182,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {
  }
  }
 
- private async getNotebookServerOptions(options: GetServerOptions): Promise<INotebookServerOptions> {
+ private getNotebookServerOptions(options: GetServerOptions): INotebookServerOptions {
  if (options.localJupyter) {
  return {
  resource: options.resource,
@@ -192,18 +191,7 @@ export class NotebookServerProvider implements IJupyterServerProvider {
  };
  }
 
- // Since there's one server per session, don't use a resource to figure out these settings
- const savedList = await this.serverUriStorage.getSavedUriList();
- let uri = !options.localJupyter
- ? savedList.find((item) => computeUriHash(item.uri) === options.serverId)?.uri
- : undefined;
-
- if (!uri) {
- throw new Error('Remote Jupyter Server connection not provided');
- }
-
  return {
- uri,
  serverId: options.serverId,
  resource: options.resource,
  ui: this.ui,

src/kernels/jupyter/types.ts

@@ -76,7 +76,6 @@ export type INotebookServerLocalOptions = {
  localJupyter: true;
 };
 export type INotebookServerRemoteOptions = {
- uri: string;
  serverId: string;
  resource: Resource;
  ui: IDisplayOptions;

src/test/kernels/jupyter/jupyterConnection.unit.test.ts

@@ -9,6 +9,7 @@ import { EventEmitter } from 'vscode';
 import { JupyterConnection } from '../../../kernels/jupyter/jupyterConnection';
 import { ServerConnectionType } from '../../../kernels/jupyter/launcher/serverConnectionType';
 import {
+ IJupyterServerUriStorage,
  IJupyterSessionManager,
  IJupyterSessionManagerFactory,
  IJupyterUriProviderRegistration
@@ -23,17 +24,20 @@ suite('Jupyter Connection', async () => {
  let sessionManagerFactory: IJupyterSessionManagerFactory;
  let serverConnectionType: ServerConnectionType;
  let sessionManager: IJupyterSessionManager;
+ let serverUriStorage: IJupyterServerUriStorage;
  const disposables: IDisposable[] = [];
  setup(() => {
  registrationPicker = mock<IJupyterUriProviderRegistration>();
  sessionManagerFactory = mock<IJupyterSessionManagerFactory>();
  serverConnectionType = mock<ServerConnectionType>();
  sessionManager = mock<IJupyterSessionManager>();
+ serverUriStorage = mock<IJupyterServerUriStorage>();
  jupyterConnection = new JupyterConnection(
  instance(registrationPicker),
  instance(sessionManagerFactory),
  disposables,
- instance(serverConnectionType)
+ instance(serverConnectionType),
+ instance(serverUriStorage)
  );
 
  (instance(sessionManager) as any).then = undefined;