diff --git a/src/kernels/jupyter/jupyterConnection.ts b/src/kernels/jupyter/jupyterConnection.ts index 70df67b7e83..c745b994603 100644 --- a/src/kernels/jupyter/jupyterConnection.ts +++ b/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 { + 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 { - return this.validateRemoteConnection(await this.createConnectionInfo(uri)); - } private async validateRemoteConnection(connection: IJupyterConnection): Promise { let sessionManager: IJupyterSessionManager | undefined = undefined; diff --git a/src/kernels/jupyter/launcher/jupyterExecution.ts b/src/kernels/jupyter/launcher/jupyterExecution.ts index da2b951b720..38ea80aac1a 100644 --- a/src/kernels/jupyter/launcher/jupyterExecution.ts +++ b/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); } } diff --git a/src/kernels/jupyter/launcher/liveshare/serverCache.ts b/src/kernels/jupyter/launcher/liveshare/serverCache.ts index 2653c8758d6..ffb4ed885ab 100644 --- a/src/kernels/jupyter/launcher/liveshare/serverCache.ts +++ b/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};`; } } diff --git a/src/kernels/jupyter/launcher/notebookServerProvider.ts b/src/kernels/jupyter/launcher/notebookServerProvider.ts index 32e6e7f316f..ffd690de8f9 100644 --- a/src/kernels/jupyter/launcher/notebookServerProvider.ts +++ b/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 { - 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 { + 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, diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts index ece31fd277c..4e62d784736 100644 --- a/src/kernels/jupyter/types.ts +++ b/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; diff --git a/src/test/kernels/jupyter/jupyterConnection.unit.test.ts b/src/test/kernels/jupyter/jupyterConnection.unit.test.ts index df46f8a06b0..917c68975dd 100644 --- a/src/test/kernels/jupyter/jupyterConnection.unit.test.ts +++ b/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(); sessionManagerFactory = mock(); serverConnectionType = mock(); sessionManager = mock(); + serverUriStorage = mock(); jupyterConnection = new JupyterConnection( instance(registrationPicker), instance(sessionManagerFactory), disposables, - instance(serverConnectionType) + instance(serverConnectionType), + instance(serverUriStorage) ); (instance(sessionManager) as any).then = undefined;