From 52d006c63028ea277a816e4e0991de7a78f42d1c Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 May 2023 14:55:13 +1000 Subject: [PATCH 1/2] Minor refactoring and comments --- src/gdpr.ts | 13 +++ src/kernels/errors/kernelErrorHandler.ts | 12 +-- .../errors/kernelErrorHandler.unit.test.ts | 24 ++--- .../execution/lastCellExecutionTracker.ts | 2 +- .../jupyter/connection/jupyterConnection.ts | 72 +++----------- .../connection/jupyterConnection.unit.test.ts | 8 +- .../connection/jupyterPasswordConnect.ts | 2 +- .../jupyterRemoteCachedKernelValidator.ts | 2 +- ...upyterUriProviderRegistration.unit.test.ts | 10 +- .../connection/jupyterUriProviderWrapper.ts | 32 ++++++- .../liveRemoteKernelConnectionTracker.ts | 2 +- ...RemoteKernelConnectionTracker.unit.test.ts | 4 +- .../remoteJupyterServerMruUpdate.ts | 5 +- .../jupyter/connection/serverSelector.ts | 14 +-- .../connection/serverSelector.unit.test.ts | 2 +- .../connection/serverSelectorCommand.ts | 4 +- .../jupyter/connection/serverUriStorage.ts | 95 ++++++------------- .../finder/remoteKernelFinder.unit.test.ts | 5 - .../finder/remoteKernelFinderController.ts | 6 +- src/kernels/jupyter/serviceRegistry.node.ts | 1 - src/kernels/jupyter/serviceRegistry.web.ts | 1 - src/kernels/jupyter/types.ts | 26 ++--- src/kernels/kernelAutoReConnectMonitor.ts | 33 ++++--- .../kernelAutoReConnectMonitor.unit.test.ts | 8 +- src/kernels/kernelProvider.node.ts | 2 +- src/kernels/kernelProvider.web.ts | 2 +- .../contributedKerneFinder.node.unit.test.ts | 2 +- src/kernels/raw/session/zeromq.node.ts | 37 +++++++- .../controllers/connectionDisplayData.ts | 2 +- .../controllers/controllerRegistration.ts | 6 +- .../controllerRegistration.unit.test.ts | 4 +- .../notebookKernelSourceSelector.ts | 4 +- .../remoteKernelControllerWatcher.ts | 8 +- ...remoteKernelControllerWatcher.unit.test.ts | 10 +- src/platform/common/constants.ts | 1 + src/standalone/api/api.ts | 2 +- src/standalone/api/extension.d.ts | 4 - .../userServerUrlProvider.ts | 2 +- src/telemetry.ts | 41 ++++++++ .../ms-ai-tools-test/src/serverPicker.ts | 10 +- .../ms-ai-tools-test/src/typings/jupyter.d.ts | 1 - src/test/datascience/notebook/helper.ts | 2 +- .../notebook/kernelSelection.vscode.test.ts | 2 +- .../remoteNotebookEditor.vscode.test.ts | 2 +- 44 files changed, 257 insertions(+), 270 deletions(-) diff --git a/src/gdpr.ts b/src/gdpr.ts index ff28ca40385..83091315b53 100644 --- a/src/gdpr.ts +++ b/src/gdpr.ts @@ -449,6 +449,18 @@ "${include}": [ "${F1}" + ] + } + */ +//Telemetry.JupyterServerProviderResponseApi +/* __GDPR__ + "DATASCIENCE.JUPYTER_SERVER_PROVIDER_RESPONSE_API" : { + "providerId": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"","owner":"donjayamanne"}, + "extensionId": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Extension Id that's attempting to use the API.","owner":"donjayamanne"}, + "pemUsed": {"classification":"PublicNonPersonalData","purpose":"FeatureInsight","comment":"Name of the API member used.","owner":"donjayamanne"}, + "${include}": [ + "${F1}" + ] } */ @@ -1458,6 +1470,7 @@ "distro_version_id": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"Linux distro version id.","owner":"donjayamanne"}, "errorMessage": {"classification":"CallstackOrException","purpose":"FeatureInsight","comment":"Error message when azure build module fails to load.","owner":"donjayamanne"}, "fallbackErrorMessage": {"classification":"CallstackOrException","purpose":"FeatureInsight","comment":"Error message when fallback module fails to load.","owner":"donjayamanne"}, + "zmqBinaries": {"classification":"CallstackOrException","purpose":"FeatureInsight","comment":"List of binaries found on disc.","owner":"donjayamanne"}, "${include}": [ "${F1}" diff --git a/src/kernels/errors/kernelErrorHandler.ts b/src/kernels/errors/kernelErrorHandler.ts index 1d6fbb15073..47f74ed456f 100644 --- a/src/kernels/errors/kernelErrorHandler.ts +++ b/src/kernels/errors/kernelErrorHandler.ts @@ -237,7 +237,7 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle error: RemoteJupyterServerUriProviderError, errorContext?: KernelAction ) { - const savedList = await this.serverUriStorage.getSavedUriList(); + const savedList = await this.serverUriStorage.getMRUs(); const message = error.originalError?.message || error.message; const serverId = error.serverId; const displayName = savedList.find((item) => item.serverId === serverId)?.displayName; @@ -253,7 +253,7 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle error: RemoteJupyterServerConnectionError, errorContext?: KernelAction ) { - const savedList = await this.serverUriStorage.getSavedUriList(); + const savedList = await this.serverUriStorage.getMRUs(); const message = error.originalError.message || ''; const serverId = error.serverId; const displayName = savedList.find((item) => item.serverId === serverId)?.displayName; @@ -305,7 +305,7 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle const handles = await provider.getHandles(); if (!handles.includes(idAndHandle.handle)) { - await this.serverUriStorage.removeUri(uri); + await this.serverUriStorage.removeMRU(uri); } return true; } catch (_ex) { @@ -366,7 +366,7 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle err instanceof InvalidRemoteJupyterServerUriHandleError ) { this.sendKernelTelemetry(err, errorContext, resource, err.category); - const savedList = await this.serverUriStorage.getSavedUriList(); + const savedList = await this.serverUriStorage.getMRUs(); const message = err instanceof InvalidRemoteJupyterServerUriHandleError ? '' @@ -402,12 +402,12 @@ export abstract class DataScienceErrorHandler implements IDataScienceErrorHandle switch (selection) { case DataScience.removeRemoteJupyterConnectionButtonText: { // Start with saved list. - const uriList = await this.serverUriStorage.getSavedUriList(); + const uriList = await this.serverUriStorage.getMRUs(); // Remove this uri if already found (going to add again with a new time) const item = uriList.find((f) => f.serverId === serverId); if (item) { - await this.serverUriStorage.removeUri(item.uri); + await this.serverUriStorage.removeMRU(item.uri); } // Wait until all of the remote controllers associated with this server have been removed. return KernelInterpreterDependencyResponse.cancel; diff --git a/src/kernels/errors/kernelErrorHandler.unit.test.ts b/src/kernels/errors/kernelErrorHandler.unit.test.ts index 8b9a6adc113..a0f4c77ca0e 100644 --- a/src/kernels/errors/kernelErrorHandler.unit.test.ts +++ b/src/kernels/errors/kernelErrorHandler.unit.test.ts @@ -794,7 +794,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d }, serverId }); - when(uriStorage.getSavedUriList()).thenResolve([]); + when(uriStorage.getMRUs()).thenResolve([]); when( applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything()) ).thenResolve(); @@ -816,7 +816,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d DataScience.selectDifferentKernel ) ).once(); - verify(uriStorage.removeUri(uri)).never(); + verify(uriStorage.removeMRU(uri)).never(); }); test('Display error when connection to remote jupyter server fails due to 3rd party extension', async () => { const uri = generateUriFromRemoteProvider('1', 'a'); @@ -833,7 +833,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d }, serverId }); - when(uriStorage.getSavedUriList()).thenResolve([{ time: 1, uri, serverId, displayName: 'Hello Server' }]); + when(uriStorage.getMRUs()).thenResolve([{ time: 1, uri, serverId, displayName: 'Hello Server' }]); when( applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything()) ).thenResolve(); @@ -855,7 +855,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d DataScience.selectDifferentKernel ) ).once(); - verify(uriStorage.removeUri(uri)).never(); + verify(uriStorage.removeMRU(uri)).never(); }); test('Remove remote Uri if user choses to do so, when connection to remote jupyter server fails', async () => { const uri = 'http://hello:1234/jupyter'; @@ -875,8 +875,8 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d when( applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything()) ).thenResolve(DataScience.removeRemoteJupyterConnectionButtonText as any); - when(uriStorage.removeUri(anything())).thenResolve(); - when(uriStorage.getSavedUriList()).thenResolve([ + when(uriStorage.removeMRU(anything())).thenResolve(); + when(uriStorage.getMRUs()).thenResolve([ { time: 1, serverId: 'foobar', uri: 'one' }, { uri, serverId, time: 2 } ]); @@ -888,8 +888,8 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d 'jupyterExtension' ); assert.strictEqual(result, KernelInterpreterDependencyResponse.cancel); - verify(uriStorage.removeUri(uri)).once(); - verify(uriStorage.getSavedUriList()).atLeast(1); + verify(uriStorage.removeMRU(uri)).once(); + verify(uriStorage.getMRUs()).atLeast(1); }); test('Change remote Uri if user choses to do so, when connection to remote jupyter server fails', async () => { const uri = 'http://hello:1234/jupyter'; @@ -906,7 +906,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d }, serverId }); - when(uriStorage.getSavedUriList()).thenResolve([]); + when(uriStorage.getMRUs()).thenResolve([]); when( applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything()) ).thenResolve(DataScience.changeRemoteJupyterConnectionButtonText as any); @@ -919,7 +919,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d 'jupyterExtension' ); assert.strictEqual(result, KernelInterpreterDependencyResponse.cancel); - verify(uriStorage.removeUri(uri)).never(); + verify(uriStorage.removeMRU(uri)).never(); }); test('Select different kernel user choses to do so, when connection to remote jupyter server fails', async () => { const uri = 'http://hello:1234/jupyter'; @@ -936,7 +936,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d }, serverId }); - when(uriStorage.getSavedUriList()).thenResolve([]); + when(uriStorage.getMRUs()).thenResolve([]); when( applicationShell.showErrorMessage(anything(), anything(), anything(), anything(), anything()) ).thenResolve(DataScience.selectDifferentKernel as any); @@ -948,7 +948,7 @@ Failed to run jupyter as observable with args notebook --no-browser --notebook-d 'jupyterExtension' ); assert.strictEqual(result, KernelInterpreterDependencyResponse.selectDifferentKernel); - verify(uriStorage.removeUri(uri)).never(); + verify(uriStorage.removeMRU(uri)).never(); }); function verifyErrorMessage(message: string, linkInfo?: string) { message = message.includes('command:jupyter.viewOutput') diff --git a/src/kernels/execution/lastCellExecutionTracker.ts b/src/kernels/execution/lastCellExecutionTracker.ts index 44ac602bd84..7d6fc7f750b 100644 --- a/src/kernels/execution/lastCellExecutionTracker.ts +++ b/src/kernels/execution/lastCellExecutionTracker.ts @@ -31,7 +31,7 @@ export class LastCellExecutionTracker extends Disposables implements IExtensionS disposables.push(this); } public activate(): void { - this.serverStorage.onDidRemoveUris(this.onDidRemoveServerUris, this, this.disposables); + this.serverStorage.onDidRemove(this.onDidRemoveServerUris, this, this.disposables); } private getStateKey(serverId: string) { return `LAST_EXECUTED_CELL_${serverId}`; diff --git a/src/kernels/jupyter/connection/jupyterConnection.ts b/src/kernels/jupyter/connection/jupyterConnection.ts index 6522ddbdddc..0a35dd268c0 100644 --- a/src/kernels/jupyter/connection/jupyterConnection.ts +++ b/src/kernels/jupyter/connection/jupyterConnection.ts @@ -2,8 +2,6 @@ // Licensed under the MIT License. import { inject, injectable } from 'inversify'; -import { IExtensionSyncActivationService } from '../../../platform/activation/types'; -import { IDisposableRegistry } from '../../../platform/common/types'; import { noop } from '../../../platform/common/utils/misc'; import { RemoteJupyterServerUriProviderError } from '../../errors/remoteJupyterServerUriProviderError'; import { BaseError } from '../../../platform/errors/types'; @@ -15,7 +13,6 @@ import { generateUriFromRemoteProvider } from '../jupyterUtils'; import { - IJupyterServerUri, IJupyterServerUriStorage, IJupyterSessionManager, IJupyterSessionManagerFactory, @@ -26,37 +23,14 @@ import { * Creates IJupyterConnection objects for URIs and 3rd party handles/ids. */ @injectable() -export class JupyterConnection implements IExtensionSyncActivationService { - private uriToJupyterServerUri = new Map(); - private pendingTimeouts: (NodeJS.Timeout | number)[] = []; +export class JupyterConnection { constructor( @inject(IJupyterUriProviderRegistration) private readonly jupyterPickerRegistration: IJupyterUriProviderRegistration, @inject(IJupyterSessionManagerFactory) private readonly jupyterSessionManagerFactory: IJupyterSessionManagerFactory, - @inject(IDisposableRegistry) - private readonly disposables: IDisposableRegistry, @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage - ) { - disposables.push(this); - } - public activate() { - this.serverUriStorage.onDidChangeUri( - () => - // When server URI changes, clear our pending URI timeouts - this.clearTimeouts(), - this, - this.disposables - ); - } - public dispose() { - this.clearTimeouts(); - } - private clearTimeouts() { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - this.pendingTimeouts.forEach((t) => clearTimeout(t as any)); - this.pendingTimeouts = []; - } + ) {} public async createConnectionInfo(options: { serverId: string } | { uri: string }) { const uri = 'uri' in options ? options.uri : await this.getUriFromServerId(options.serverId); @@ -71,15 +45,12 @@ export class JupyterConnection implements IExtensionSyncActivationService { 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(); + const savedList = await this.serverUriStorage.getMRUs(); return savedList.find((item) => item.serverId === serverId)?.uri; } private async createConnectionInfoFromUri(uri: string) { - // Prepare our map of server URIs - await this.updateServerUri(uri); - + const server = await this.getJupyterServerUri(uri); const idAndHandle = extractJupyterServerHandleAndId(uri); - const server = this.uriToJupyterServerUri.get(uri); return createRemoteConnectionInfo(uri, server, idAndHandle?.id); } @@ -99,32 +70,19 @@ export class JupyterConnection implements IExtensionSyncActivationService { } } - private async updateServerUri(uri: string): Promise { + private async getJupyterServerUri(uri: string) { const idAndHandle = extractJupyterServerHandleAndId(uri); - if (idAndHandle) { - try { - const serverUri = await this.jupyterPickerRegistration.getJupyterServerUri( - idAndHandle.id, - idAndHandle.handle - ); - this.uriToJupyterServerUri.set(uri, serverUri); - // See if there's an expiration date - if (serverUri.expiration) { - const timeoutInMS = serverUri.expiration.getTime() - Date.now(); - // Week seems long enough (in case the expiration is ridiculous) - if (timeoutInMS > 0 && timeoutInMS < 604800000) { - this.pendingTimeouts.push(setTimeout(() => this.updateServerUri(uri).catch(noop), timeoutInMS)); - } - } - } catch (ex) { - if (ex instanceof BaseError) { - throw ex; - } - const serverId = await computeServerId( - generateUriFromRemoteProvider(idAndHandle.id, idAndHandle.handle) - ); - throw new RemoteJupyterServerUriProviderError(idAndHandle.id, idAndHandle.handle, ex, serverId); + if (!idAndHandle) { + return; + } + try { + return await this.jupyterPickerRegistration.getJupyterServerUri(idAndHandle.id, idAndHandle.handle); + } catch (ex) { + if (ex instanceof BaseError) { + throw ex; } + const serverId = await computeServerId(generateUriFromRemoteProvider(idAndHandle.id, idAndHandle.handle)); + throw new RemoteJupyterServerUriProviderError(idAndHandle.id, idAndHandle.handle, ex, serverId); } } } diff --git a/src/kernels/jupyter/connection/jupyterConnection.unit.test.ts b/src/kernels/jupyter/connection/jupyterConnection.unit.test.ts index 105f8af217d..f73a1aa432d 100644 --- a/src/kernels/jupyter/connection/jupyterConnection.unit.test.ts +++ b/src/kernels/jupyter/connection/jupyterConnection.unit.test.ts @@ -47,7 +47,6 @@ suite('Jupyter Connection', async () => { jupyterConnection = new JupyterConnection( instance(registrationPicker), instance(sessionManagerFactory), - disposables, instance(serverUriStorage) ); @@ -56,17 +55,12 @@ suite('Jupyter Connection', async () => { const serverConnectionChangeEvent = new EventEmitter(); disposables.push(serverConnectionChangeEvent); - when(serverUriStorage.onDidChangeUri).thenReturn(serverConnectionChangeEvent.event); - - jupyterConnection.activate(); + when(serverUriStorage.onDidChange).thenReturn(serverConnectionChangeEvent.event); }); teardown(() => { disposeAllDisposables(disposables); }); - test('Ensure event handler is added', () => { - verify(serverUriStorage.onDidChangeUri).once(); - }); test('Validation will result in fetching kernels and kernelspecs', async () => { const uri = 'http://localhost:8888/?token=1234'; when(sessionManager.dispose()).thenResolve(); diff --git a/src/kernels/jupyter/connection/jupyterPasswordConnect.ts b/src/kernels/jupyter/connection/jupyterPasswordConnect.ts index ad2a7a0a4ca..d97798d0c76 100644 --- a/src/kernels/jupyter/connection/jupyterPasswordConnect.ts +++ b/src/kernels/jupyter/connection/jupyterPasswordConnect.ts @@ -39,7 +39,7 @@ export class JupyterPasswordConnect implements IJupyterPasswordConnect { @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry ) { // Sign up to see if servers are removed from our uri storage list - this.serverUriStorage.onDidRemoveUris(this.onDidRemoveUris, this, this.disposables); + this.serverUriStorage.onDidRemove(this.onDidRemoveUris, this, this.disposables); } private static _prompt?: Deferred; public static get prompt(): Promise | undefined { diff --git a/src/kernels/jupyter/connection/jupyterRemoteCachedKernelValidator.ts b/src/kernels/jupyter/connection/jupyterRemoteCachedKernelValidator.ts index 73942da344d..f79dabc1977 100644 --- a/src/kernels/jupyter/connection/jupyterRemoteCachedKernelValidator.ts +++ b/src/kernels/jupyter/connection/jupyterRemoteCachedKernelValidator.ts @@ -30,7 +30,7 @@ export class JupyterRemoteCachedKernelValidator implements IJupyterRemoteCachedK return false; } const providersPromise = this.providerRegistration.getProviders(); - const currentList = await this.uriStorage.getSavedUriList(); + const currentList = await this.uriStorage.getMRUs(); const item = currentList.find((item) => item.serverId === kernel.serverId); if (!item) { // Server has been removed and we have some old cached data. diff --git a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts index 2d18c3d7e31..9bd7446bff4 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderRegistration.unit.test.ts @@ -33,20 +33,12 @@ class MockProvider implements IJupyterUriProvider { } public async getServerUri(handle: string): Promise { if (handle === '1') { - const currentDate = new Date(); return { // eslint-disable-next-line baseUrl: 'http://foobar:3000', token: '', displayName: 'dummy', - authorizationHeader: { Bearer: this.currentBearer.toString() }, - expiration: new Date( - currentDate.getFullYear(), - currentDate.getMonth(), - undefined, - currentDate.getHours(), - currentDate.getMinutes() + 1 // Expire after one minute - ) + authorizationHeader: { Bearer: this.currentBearer.toString() } }; } diff --git a/src/kernels/jupyter/connection/jupyterUriProviderWrapper.ts b/src/kernels/jupyter/connection/jupyterUriProviderWrapper.ts index b34944514a7..bf0270a3a79 100644 --- a/src/kernels/jupyter/connection/jupyterUriProviderWrapper.ts +++ b/src/kernels/jupyter/connection/jupyterUriProviderWrapper.ts @@ -5,7 +5,9 @@ import * as vscode from 'vscode'; import { IDisposableRegistry } from '../../../platform/common/types'; import * as localize from '../../../platform/common/utils/localize'; import { IJupyterUriProvider, JupyterServerUriHandle, IJupyterServerUri } from '../types'; +import { Telemetry, sendTelemetryEvent } from '../../../telemetry'; +const handlesForWhichWeHaveSentTelemetry = new Set(); /** * This class wraps an IJupyterUriProvider provided by another extension. It allows us to show * extra data on the other extension's UI. @@ -17,7 +19,7 @@ export class JupyterUriProviderWrapper implements IJupyterUriProvider { constructor( private readonly provider: IJupyterUriProvider, - private packageName: string, + private extensionId: string, disposables: IDisposableRegistry ) { if (provider.onDidChangeHandles) { @@ -61,7 +63,7 @@ export class JupyterUriProviderWrapper implements IJupyterUriProvider { return { ...q, // Add the package name onto the description - description: localize.DataScience.uriProviderDescriptionFormat(q.description || '', this.packageName), + description: localize.DataScience.uriProviderDescriptionFormat(q.description || '', this.extensionId), original: q }; }); @@ -81,7 +83,29 @@ export class JupyterUriProviderWrapper implements IJupyterUriProvider { return this.provider.handleQuickPick(item, back); } - public getServerUri(handle: JupyterServerUriHandle): Promise { - return this.provider.getServerUri(handle); + public async getServerUri(handle: JupyterServerUriHandle): Promise { + const server = await this.provider.getServerUri(handle); + if (!this.id.startsWith('_builtin') && !handlesForWhichWeHaveSentTelemetry.has(handle)) { + handlesForWhichWeHaveSentTelemetry.add(handle); + // Need this info to try and remove some of the properties from the API. + // Before we do that we need to determine what extensions are using which properties. + const pemUsed: (keyof IJupyterServerUri)[] = []; + Object.keys(server).forEach((k) => { + const value = server[k as keyof IJupyterServerUri]; + if (!value) { + return; + } + if (typeof value === 'object' && Object.keys(value).length === 0 && !(value instanceof Date)) { + return; + } + pemUsed.push(k as keyof IJupyterServerUri); + }); + sendTelemetryEvent(Telemetry.JupyterServerProviderResponseApi, undefined, { + providerId: this.id, + extensionId: this.extensionId, + pemUsed + }); + } + return server; } } diff --git a/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.ts b/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.ts index 4515c32aa3a..780a3430b35 100644 --- a/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.ts +++ b/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.ts @@ -35,7 +35,7 @@ export class LiveRemoteKernelConnectionUsageTracker mementoKeyToTrackRemoveKernelUrisAndSessionsUsedByResources, {} ); - this.uriStorage.onDidRemoveUris(this.onDidRemoveUris, this, this.disposables); + this.uriStorage.onDidRemove(this.onDidRemoveUris, this, this.disposables); } public wasKernelUsed(connection: LiveRemoteKernelConnectionMetadata) { diff --git a/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.unit.test.ts b/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.unit.test.ts index 9f3bc31fe0a..91cfda97173 100644 --- a/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.unit.test.ts +++ b/src/kernels/jupyter/connection/liveRemoteKernelConnectionTracker.unit.test.ts @@ -96,7 +96,7 @@ suite('Live kernel Connection Tracker', async () => { memento = mock(); onDidRemoveUris = new EventEmitter(); disposables.push(onDidRemoveUris); - when(serverUriStorage.onDidRemoveUris).thenReturn(onDidRemoveUris.event); + when(serverUriStorage.onDidRemove).thenReturn(onDidRemoveUris.event); tracker = new LiveRemoteKernelConnectionUsageTracker( disposables, instance(serverUriStorage), @@ -109,7 +109,7 @@ suite('Live kernel Connection Tracker', async () => { test('Ensure event handler is added', () => { tracker.activate(); - verify(serverUriStorage.onDidRemoveUris).once(); + verify(serverUriStorage.onDidRemove).once(); }); test('Kernel connection is not used if memento is empty', async () => { when(memento.get(anything(), anything())).thenCall((_, defaultValue) => defaultValue); diff --git a/src/kernels/jupyter/connection/remoteJupyterServerMruUpdate.ts b/src/kernels/jupyter/connection/remoteJupyterServerMruUpdate.ts index df85f40945f..8f667d35b32 100644 --- a/src/kernels/jupyter/connection/remoteJupyterServerMruUpdate.ts +++ b/src/kernels/jupyter/connection/remoteJupyterServerMruUpdate.ts @@ -43,10 +43,9 @@ export class RemoteJupyterServerMruUpdate implements IExtensionSyncActivationSer clearTimeout(timeout); } if (kernel.status === 'idle' && !kernel.disposed && !kernel.disposing) { - const now = Date.now(); const timeout = setTimeout(() => { // Log this remote URI into our MRU list - this.serverStorage.addServerToUriList(connection.serverId, now).catch(noop); + this.serverStorage.updateMRU(connection.serverId).catch(noop); }, INTERVAL_IN_SECONDS_TO_UPDATE_MRU); this.monitoredKernels.set(kernel, timeout); this.disposables.push(new Disposable(() => clearTimeout(timeout))); @@ -57,6 +56,6 @@ export class RemoteJupyterServerMruUpdate implements IExtensionSyncActivationSer ); // Log this remote URI into our MRU list - this.serverStorage.addServerToUriList(connection.serverId, Date.now()).catch(noop); + this.serverStorage.updateMRU(connection.serverId).catch(noop); } } diff --git a/src/kernels/jupyter/connection/serverSelector.ts b/src/kernels/jupyter/connection/serverSelector.ts index b8f70b48c25..523e48107db 100644 --- a/src/kernels/jupyter/connection/serverSelector.ts +++ b/src/kernels/jupyter/connection/serverSelector.ts @@ -95,16 +95,10 @@ export class JupyterServerSelector { @inject(IDisposableRegistry) readonly disposableRegistry: IDisposableRegistry ) {} - public async setJupyterURIToRemote( - userURI: string, - ignoreValidation?: boolean, - displayName?: string - ): Promise { + public async addJupyterServer(userURI: string): Promise { // Double check this server can be connected to. Might need a password, might need a allowUnauthorized try { - if (!ignoreValidation) { - await this.jupyterConnection.validateRemoteUri(userURI); - } + await this.jupyterConnection.validateRemoteUri(userURI); } catch (err) { if (JupyterSelfCertsError.isSelfCertsError(err)) { sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter); @@ -127,9 +121,7 @@ export class JupyterServerSelector { } const connection = await this.jupyterConnection.createConnectionInfo({ uri: userURI }); - displayName && (connection.displayName = displayName); - - await this.serverUriStorage.setUriToRemote(userURI, connection.displayName); + await this.serverUriStorage.addMRU(userURI, connection.displayName); // Indicate setting a jupyter URI to a remote setting. Check if an azure remote or not sendTelemetryEvent(Telemetry.SetJupyterURIToUserSpecified, undefined, { diff --git a/src/kernels/jupyter/connection/serverSelector.unit.test.ts b/src/kernels/jupyter/connection/serverSelector.unit.test.ts index 18e91424b66..c2ec9785cd5 100644 --- a/src/kernels/jupyter/connection/serverSelector.unit.test.ts +++ b/src/kernels/jupyter/connection/serverSelector.unit.test.ts @@ -46,6 +46,6 @@ suite('Server Selector Command', () => { handler(); - verify(serverSelector.setJupyterURIToRemote('http://localhost:1234/')).once(); + verify(serverSelector.addJupyterServer('http://localhost:1234/')).once(); }); }); diff --git a/src/kernels/jupyter/connection/serverSelectorCommand.ts b/src/kernels/jupyter/connection/serverSelectorCommand.ts index ccfd5f6730d..8f4911ef6c9 100644 --- a/src/kernels/jupyter/connection/serverSelectorCommand.ts +++ b/src/kernels/jupyter/connection/serverSelectorCommand.ts @@ -39,11 +39,11 @@ export class JupyterServerSelectorCommand implements IExtensionSyncActivationSer traceInfo(`Setting Jupyter Server URI to remote: ${source}`); // Set the uri directly - await this.serverSelector.setJupyterURIToRemote(source.toString(true)); + await this.serverSelector.addJupyterServer(source.toString(true)); } } private async clearJupyterUris(): Promise { - return this.serverUriStorage.clearUriList(); + return this.serverUriStorage.clearMRU(); } } diff --git a/src/kernels/jupyter/connection/serverUriStorage.ts b/src/kernels/jupyter/connection/serverUriStorage.ts index f6058ce59a4..33324ec87de 100644 --- a/src/kernels/jupyter/connection/serverUriStorage.ts +++ b/src/kernels/jupyter/connection/serverUriStorage.ts @@ -1,8 +1,8 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import { EventEmitter, Memento } from 'vscode'; import { inject, injectable, named } from 'inversify'; -import { Event, EventEmitter, Memento } from 'vscode'; import { IWorkspaceService, IEncryptedStorage, @@ -10,88 +10,59 @@ 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, - IsWebExtension, - IConfigurationService -} from '../../../platform/common/types'; +import { ICryptoUtils, IMemento, GLOBAL_MEMENTO } from '../../../platform/common/types'; import { traceInfoIfCI, traceVerbose } from '../../../platform/logging'; import { computeServerId, extractJupyterServerHandleAndId } from '../jupyterUtils'; import { IJupyterServerUriEntry, IJupyterServerUriStorage, IJupyterUriProviderRegistration } from '../types'; /** - * Class for storing Jupyter Server URI values + * Class for storing Jupyter Server URI values, also manages the MRU list of the servers/urls. */ @injectable() export class JupyterServerUriStorage implements IJupyterServerUriStorage { private lastSavedList?: Promise; private _onDidChangeUri = new EventEmitter(); - public get onDidChangeUri() { + public get onDidChange() { return this._onDidChangeUri.event; } private _onDidRemoveUris = new EventEmitter(); - public get onDidRemoveUris() { + public get onDidRemove() { return this._onDidRemoveUris.event; } private _onDidAddUri = new EventEmitter(); - public get onDidAddUri() { + public get onDidAdd() { return this._onDidAddUri.event; } - public get onDidChangeConnectionType(): Event { - return this._onDidChangeUri.event; - } 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(IsWebExtension) readonly isWebExtension: boolean, - @inject(IConfigurationService) readonly configService: IConfigurationService, @inject(IJupyterUriProviderRegistration) private readonly jupyterPickerRegistration: IJupyterUriProviderRegistration ) {} - public async addServerToUriList(serverId: string, time: number) { - // Start with saved list. - const uriList = await this.getSavedUriList(); - - // Check if we have already found a display name for this server - const existingEntry = uriList.find((entry) => { - return entry.serverId === serverId; - }); + public async updateMRU(serverId: string) { + const uriList = await this.getMRUs(); + const existingEntry = uriList.find((entry) => entry.serverId === serverId); if (!existingEntry) { throw new Error(`Uri not found for Server Id ${serverId}`); } - await this.addToUriList(existingEntry.uri, time, existingEntry.displayName || ''); + await this.addToUriList(existingEntry.uri, existingEntry.displayName || ''); } - private async addToUriList(uri: string, time: number, displayName: string) { - // Uri list is saved partially in the global memento and partially in encrypted storage - - // Start with saved list. - const uriList = await this.getSavedUriList(); - - // Compute server id for saving in the list - const serverId = await computeServerId(uri); + private async addToUriList(uri: string, displayName: string) { + const [uriList, serverId] = await Promise.all([this.getMRUs(), computeServerId(uri)]); // Check if we have already found a display name for this server - const existingEntry = uriList.find((entry) => { - return entry.serverId === serverId; - }); - if (existingEntry && existingEntry.displayName) { - displayName = existingEntry.displayName; - } + displayName = uriList.find((entry) => entry.serverId === serverId)?.displayName || displayName || uri; // Remove this uri if already found (going to add again with a new time) - const editedList = uriList.filter((f, i) => { - return f.uri !== uri && i < Settings.JupyterServerUriListMax - 1; - }); + const editedList = uriList.filter((f, i) => f.uri !== uri && i < Settings.JupyterServerUriListMax - 1); // Add this entry into the last. - const entry = { uri, time, serverId, displayName: displayName || uri, isValidated: true }; + const entry = { uri, time: Date.now(), serverId, displayName, isValidated: true }; editedList.push(entry); // Signal that we added in the entry @@ -99,12 +70,10 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage { return this.updateMemento(editedList); } - public async removeUri(uri: string) { - // Start with saved list. - const uriList = await this.getSavedUriList(); + public async removeMRU(uri: string) { + const uriList = await this.getMRUs(); - const editedList = uriList.filter((f) => f.uri !== uri); - await this.updateMemento(editedList); + await this.updateMemento(uriList.filter((f) => f.uri !== uri)); const removedItem = uriList.find((f) => f.uri === uri); if (removedItem) { this._onDidRemoveUris.fire([removedItem]); @@ -112,9 +81,7 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage { } private async updateMemento(editedList: IJupyterServerUriEntry[]) { // Sort based on time. Newest time first - const sorted = editedList.sort((a, b) => { - return b.time - a.time; - }); + const sorted = editedList.sort((a, b) => b.time - a.time); // Transform the sorted into just indexes. Uris can't show up in // non encrypted storage (so remove even the display name) @@ -144,7 +111,7 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage { blob ); } - public async getSavedUriList(): Promise { + public async getMRUs(): Promise { if (this.lastSavedList) { return this.lastSavedList; } @@ -216,8 +183,8 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage { return this.lastSavedList; } - public async clearUriList(): Promise { - const uriList = await this.getSavedUriList(); + public async clearMRU(): Promise { + const uriList = await this.getMRUs(); this.lastSavedList = Promise.resolve([]); // Clear out memento and encrypted storage await this.globalMemento.update(Settings.JupyterServerUriList, []); @@ -228,23 +195,17 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage { ); // Notify out that we've removed the list to clean up controller entries, passwords, ect - this._onDidRemoveUris.fire( - uriList.map((uriListItem) => { - return uriListItem; - }) - ); + this._onDidRemoveUris.fire(uriList); } - public async getUriForServer(id: string): Promise { - const savedList = await this.getSavedUriList(); - const uriItem = savedList.find((item) => item.serverId === id); - - return uriItem; + public async getMRU(id: string): Promise { + const savedList = await this.getMRUs(); + return savedList.find((item) => item.serverId === id); } - public async setUriToRemote(uri: string, displayName: string): Promise { + public async addMRU(uri: string, displayName: string): Promise { traceInfoIfCI(`setUri: ${uri}`); // display name is wrong here - await this.addToUriList(uri, Date.now(), displayName ?? uri); + await this.addToUriList(uri, displayName ?? uri); this._onDidChangeUri.fire(); // Needs to happen as soon as we change so that dependencies update synchronously // Save in the storage (unique account per workspace) diff --git a/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts b/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts index c051aefeb9c..ac034d15cd4 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts @@ -25,7 +25,6 @@ import { IJupyterKernel, IJupyterRemoteCachedKernelValidator, IJupyterSessionMan import { KernelFinder } from '../../kernelFinder'; import { PythonExtensionChecker } from '../../../platform/api/pythonApi'; import { IFileSystemNode } from '../../../platform/common/platform/types.node'; -import { JupyterServerUriStorage } from '../connection/serverUriStorage'; import { FileSystem } from '../../../platform/common/platform/fileSystem.node'; import { IApplicationEnvironment } from '../../../platform/common/application/types'; import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder'; @@ -129,16 +128,12 @@ suite(`Remote Kernel Finder`, () => { fs = mock(FileSystem); when(fs.delete(anything())).thenResolve(); when(fs.exists(anything())).thenResolve(true); - const serverUriStorage = mock(JupyterServerUriStorage); const serverEntry = { uri: connInfo.baseUrl, time: Date.now(), serverId: connInfo.baseUrl, isValidated: true }; - const onDidChangeEvent = new EventEmitter(); - disposables.push(onDidChangeEvent); - when(serverUriStorage.onDidChangeConnectionType).thenReturn(onDidChangeEvent.event); cachedRemoteKernelValidator = mock(); when(cachedRemoteKernelValidator.isValid(anything())).thenResolve(true); const env = mock(); diff --git a/src/kernels/jupyter/finder/remoteKernelFinderController.ts b/src/kernels/jupyter/finder/remoteKernelFinderController.ts index a612bbd6e16..fa9b8450dfa 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinderController.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinderController.ts @@ -44,15 +44,15 @@ export class RemoteKernelFinderController implements IExtensionSyncActivationSer activate() { // Add in the URIs that we already know about this.serverUriStorage - .getSavedUriList() + .getMRUs() .then((currentServers) => { currentServers.forEach(this.createRemoteKernelFinder.bind(this)); // Check for when more URIs are added - this.serverUriStorage.onDidAddUri(this.createRemoteKernelFinder, this, this.disposables); + this.serverUriStorage.onDidAdd(this.createRemoteKernelFinder, this, this.disposables); // Also check for when a URI is removed - this.serverUriStorage.onDidRemoveUris(this.urisRemoved, this, this.disposables); + this.serverUriStorage.onDidRemove(this.urisRemoved, this, this.disposables); }) .catch(noop); } diff --git a/src/kernels/jupyter/serviceRegistry.node.ts b/src/kernels/jupyter/serviceRegistry.node.ts index c7646606ee0..31ce4cc5b8d 100644 --- a/src/kernels/jupyter/serviceRegistry.node.ts +++ b/src/kernels/jupyter/serviceRegistry.node.ts @@ -123,7 +123,6 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole serviceManager.addSingleton(IJupyterRequestCreator, JupyterRequestCreator); serviceManager.addSingleton(IJupyterRequestAgentCreator, RequestAgentCreator); serviceManager.addSingleton(JupyterConnection, JupyterConnection); - serviceManager.addBinding(JupyterConnection, IExtensionSyncActivationService); serviceManager.addSingleton( ILiveRemoteKernelConnectionUsageTracker, LiveRemoteKernelConnectionUsageTracker diff --git a/src/kernels/jupyter/serviceRegistry.web.ts b/src/kernels/jupyter/serviceRegistry.web.ts index 6a8fa9cb2b9..bd5e309b814 100644 --- a/src/kernels/jupyter/serviceRegistry.web.ts +++ b/src/kernels/jupyter/serviceRegistry.web.ts @@ -56,7 +56,6 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole serviceManager.addSingleton(IJupyterServerProvider, JupyterServerProvider); serviceManager.addSingleton(IJupyterRequestCreator, JupyterRequestCreator); serviceManager.addSingleton(JupyterConnection, JupyterConnection); - serviceManager.addBinding(JupyterConnection, IExtensionSyncActivationService); serviceManager.addSingleton( ILiveRemoteKernelConnectionUsageTracker, LiveRemoteKernelConnectionUsageTracker diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts index e78e86f565f..31b7877b37a 100644 --- a/src/kernels/jupyter/types.ts +++ b/src/kernels/jupyter/types.ts @@ -165,10 +165,6 @@ export interface IJupyterServerUri { * Authorization header to be used when connecting to the server. */ authorizationHeader: Record; - /** - * Date/time when header expires and should be refreshed. - */ - expiration?: Date; displayName: string; /** * The local directory that maps to the remote directory of the Jupyter Server. @@ -270,20 +266,18 @@ export interface IJupyterServerUriEntry { export const IJupyterServerUriStorage = Symbol('IJupyterServerUriStorage'); export interface IJupyterServerUriStorage { - readonly onDidChangeUri: Event; - readonly onDidRemoveUris: Event; - readonly onDidAddUri: Event; + readonly onDidChange: Event; + readonly onDidRemove: Event; + readonly onDidAdd: Event; /** - * Adds a server to the MRU list. - * Similar to `addToUriList` however one does not need to pass the `Uri` nor the `displayName`. - * As Uri could contain sensitive information and `displayName` would have already been setup. + * Updates MRU list marking this server as the most recently used. */ - addServerToUriList(serverId: string, time: number): Promise; - getSavedUriList(): Promise; - removeUri(uri: string): Promise; - clearUriList(): Promise; - getUriForServer(id: string): Promise; - setUriToRemote(uri: string, displayName: string): Promise; + updateMRU(serverId: string): Promise; + getMRUs(): Promise; + removeMRU(uri: string): Promise; + clearMRU(): Promise; + getMRU(serverId: string): Promise; + addMRU(uri: string, displayName: string): Promise; } export interface IBackupFile { diff --git a/src/kernels/kernelAutoReConnectMonitor.ts b/src/kernels/kernelAutoReConnectMonitor.ts index 5012eea6602..166df1517ab 100644 --- a/src/kernels/kernelAutoReConnectMonitor.ts +++ b/src/kernels/kernelAutoReConnectMonitor.ts @@ -15,7 +15,13 @@ import { Telemetry } from '../telemetry'; import { endCellAndDisplayErrorsInCell } from './execution/helpers'; import { getDisplayNameOrNameOfKernelConnection } from './helpers'; import { sendKernelTelemetryEvent } from './telemetry/sendKernelTelemetryEvent'; -import { IKernel, IKernelProvider, isLocalConnection, RemoteKernelConnectionMetadata } from './types'; +import { + IKernel, + IKernelProvider, + isLocalConnection, + isRemoteConnection, + RemoteKernelConnectionMetadata +} from './types'; import { IJupyterServerUriStorage, IJupyterUriProviderRegistration } from './jupyter/types'; import { extractJupyterServerHandleAndId } from './jupyter/jupyterUtils'; @@ -143,11 +149,8 @@ export class KernelAutoReconnectMonitor implements IExtensionSyncActivationServi const disposable = new Disposable(() => deferred.resolve()); this.kernelReconnectProgress.set(kernel, disposable); - if ( - kernel.kernelConnectionMetadata.kind === 'connectToLiveRemoteKernel' || - kernel.kernelConnectionMetadata.kind === 'startUsingRemoteKernelSpec' - ) { - const handled = await this.handleRemoteServerReinitiate(kernel, kernel.kernelConnectionMetadata); + if (isRemoteConnection(kernel.kernelConnectionMetadata)) { + const handled = await this.handleRemoteServerShutdown(kernel, kernel.kernelConnectionMetadata); if (handled) { return; @@ -173,11 +176,8 @@ export class KernelAutoReconnectMonitor implements IExtensionSyncActivationServi // If this is a connection from a uri provider (such as a remote server), then we cannot restart the kernel. // Let's request the uri provider to resolve the uri and then reconnect - if ( - kernel.kernelConnectionMetadata.kind === 'connectToLiveRemoteKernel' || - kernel.kernelConnectionMetadata.kind === 'startUsingRemoteKernelSpec' - ) { - const handled = await this.handleRemoteServerReinitiate(kernel, kernel.kernelConnectionMetadata); + if (isRemoteConnection(kernel.kernelConnectionMetadata)) { + const handled = await this.handleRemoteServerShutdown(kernel, kernel.kernelConnectionMetadata); if (handled) { return; @@ -205,11 +205,16 @@ export class KernelAutoReconnectMonitor implements IExtensionSyncActivationServi } } - private async handleRemoteServerReinitiate( + /** + * Possible the kernel died because the remote jupyter server is no longer available. + * Validate the server is still available with the jupyter providers and remove stale servers. + * @return {*} {Promise `true` if the remote server has shutdown, else `false` } + */ + private async handleRemoteServerShutdown( kernel: IKernel, metadata: RemoteKernelConnectionMetadata ): Promise { - const uriItem = await this.serverUriStorage.getUriForServer(metadata.serverId); + const uriItem = await this.serverUriStorage.getMRU(metadata.serverId); if (!uriItem) { return false; @@ -230,7 +235,7 @@ export class KernelAutoReconnectMonitor implements IExtensionSyncActivationServi const handles = await provider.getHandles(); if (!handles.includes(idAndHandle.handle)) { - await this.serverUriStorage.removeUri(uriItem.uri); + await this.serverUriStorage.removeMRU(uriItem.uri); this.kernelReconnectProgress.get(kernel)?.dispose(); this.kernelReconnectProgress.delete(kernel); } diff --git a/src/kernels/kernelAutoReConnectMonitor.unit.test.ts b/src/kernels/kernelAutoReConnectMonitor.unit.test.ts index 8f002f5beab..e02cfc172d1 100644 --- a/src/kernels/kernelAutoReConnectMonitor.unit.test.ts +++ b/src/kernels/kernelAutoReConnectMonitor.unit.test.ts @@ -62,7 +62,7 @@ suite('Kernel ReConnect Progress Message', () => { when(kernelProvider.getKernelExecution(anything())).thenReturn(instance(kernelExecution)); clock = fakeTimers.install(); jupyterServerUriStorage = mock(); - when(jupyterServerUriStorage.getSavedUriList()).thenResolve([]); + when(jupyterServerUriStorage.getMRUs()).thenResolve([]); jupyterUriProviderRegistration = mock(); disposables.push(new Disposable(() => clock.uninstall())); @@ -170,7 +170,7 @@ suite('Kernel ReConnect Failed Monitor', () => { when(kernelProvider.onDidRestartKernel).thenReturn(onDidRestartKernel.event); when(kernelProvider.getKernelExecution(anything())).thenReturn(instance(kernelExecution)); jupyterServerUriStorage = mock(); - when(jupyterServerUriStorage.getSavedUriList()).thenResolve([]); + when(jupyterServerUriStorage.getMRUs()).thenResolve([]); jupyterUriProviderRegistration = mock(); monitor = new KernelAutoReconnectMonitor( instance(appShell), @@ -326,8 +326,8 @@ suite('Kernel ReConnect Failed Monitor', () => { serverId: '1234', time: 1234 }; - when(jupyterServerUriStorage.getSavedUriList()).thenResolve([server]); - when(jupyterServerUriStorage.getUriForServer(anything())).thenResolve(server); + when(jupyterServerUriStorage.getMRUs()).thenResolve([server]); + when(jupyterServerUriStorage.getMRU(anything())).thenResolve(server); when(jupyterUriProviderRegistration.getProvider(anything())).thenResolve({ id: 'remoteUriProvider', getServerUri: (_handle) => diff --git a/src/kernels/kernelProvider.node.ts b/src/kernels/kernelProvider.node.ts index 81a98cf2ce1..a47c981d069 100644 --- a/src/kernels/kernelProvider.node.ts +++ b/src/kernels/kernelProvider.node.ts @@ -48,7 +48,7 @@ export class KernelProvider extends BaseCoreKernelProvider { @inject(IMemento) @named(WORKSPACE_MEMENTO) private readonly workspaceStorage: Memento ) { super(asyncDisposables, disposables, notebook); - disposables.push(jupyterServerUriStorage.onDidRemoveUris(this.handleUriRemoval.bind(this))); + disposables.push(jupyterServerUriStorage.onDidRemove(this.handleUriRemoval.bind(this))); } public getOrCreate(notebook: NotebookDocument, options: KernelOptions): IKernel { diff --git a/src/kernels/kernelProvider.web.ts b/src/kernels/kernelProvider.web.ts index 5e064dd214d..c612a44a947 100644 --- a/src/kernels/kernelProvider.web.ts +++ b/src/kernels/kernelProvider.web.ts @@ -47,7 +47,7 @@ export class KernelProvider extends BaseCoreKernelProvider { @inject(IMemento) @named(WORKSPACE_MEMENTO) private readonly workspaceStorage: Memento ) { super(asyncDisposables, disposables, notebook); - disposables.push(jupyterServerUriStorage.onDidRemoveUris(this.handleUriRemoval.bind(this))); + disposables.push(jupyterServerUriStorage.onDidRemove(this.handleUriRemoval.bind(this))); } public getOrCreate(notebook: NotebookDocument, options: KernelOptions): IKernel { diff --git a/src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts b/src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts index 9d213e55403..e45ad69013a 100644 --- a/src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts +++ b/src/kernels/raw/finder/contributedKerneFinder.node.unit.test.ts @@ -246,7 +246,7 @@ import { IPythonExecutionService, IPythonExecutionFactory } from '../../../platf const uriStorage = mock(); const onDidChangeEvent = new EventEmitter(); disposables.push(onDidChangeEvent); - when(uriStorage.onDidChangeUri).thenReturn(onDidChangeEvent.event); + when(uriStorage.onDidChange).thenReturn(onDidChangeEvent.event); const extensions = mock(); const trustedKernels = mock(); diff --git a/src/kernels/raw/session/zeromq.node.ts b/src/kernels/raw/session/zeromq.node.ts index e2e5999bb3e..949369760d1 100644 --- a/src/kernels/raw/session/zeromq.node.ts +++ b/src/kernels/raw/session/zeromq.node.ts @@ -31,14 +31,46 @@ export function getZeroMQ(): typeof import('zeromq') { } } } - +/** + * We need to send telemetry to understand how many users are failing to load the binaries. + * Its possible they have installed the wrong extension or the files do not exist or the like. + * This is required to ensure kernels run successfully (by understanding the cause of the failures). + */ +async function getLocalZmqBinaries() { + try { + const zmqFolder = path.join(EXTENSION_ROOT_DIR, 'out', 'node_modules', 'zeromq', 'prebuilds'); + const filesPromises = await fs.readdir(zmqFolder).then((folders) => + folders.map(async (folder) => { + const folderPath = path.join(zmqFolder, folder); + const stat = await fs.stat(folderPath); + if (stat.isDirectory()) { + return fs.readdir(folderPath).then((files) => files.map((file) => path.join(folderPath, file))); + } + return []; + }) + ); + const files = (await Promise.all(filesPromises.flat())).flat(); + return files.map((file) => + file + .substring(file.lastIndexOf('prebuilds') + 'prebuilds'.length + 1) + .replace(/\\/g, '') + .replace(/\//g, '') + ); + } catch (ex) { + traceWarning(`Failed to determine local zmq binaries.`, ex); + return ['Failed to determine local zmq binaries.']; + } +} async function sendZMQTelemetry( failed: boolean, fallbackTried: boolean = false, errorMessage = '', fallbackErrorMessage = '' ) { - const distro = await getDistroInfo().catch(() => { id: '', version_id: '' }); + const [distro, zmqBinaries] = await Promise.all([ + getDistroInfo().catch(() => { id: '', version_id: '' }), + getLocalZmqBinaries() + ]); const platformInfo = getPlatformInfo(); sendTelemetryEvent(Telemetry.ZMQSupport, undefined, { distro_id: distro.id, @@ -60,6 +92,7 @@ async function sendZMQTelemetry( armv: platformInfo.armv, zmqarch: platformInfo.zmqarch, errorMessage, + zmqBinaries, fallbackErrorMessage }); } diff --git a/src/notebooks/controllers/connectionDisplayData.ts b/src/notebooks/controllers/connectionDisplayData.ts index c505e4bdaff..b0d9726e1b0 100644 --- a/src/notebooks/controllers/connectionDisplayData.ts +++ b/src/notebooks/controllers/connectionDisplayData.ts @@ -145,7 +145,7 @@ async function getRemoteServerDisplayName( kernelConnection: RemoteKernelConnectionMetadata, serverUriStorage: IJupyterServerUriStorage ): Promise { - const savedUriList = await serverUriStorage.getSavedUriList(); + const savedUriList = await serverUriStorage.getMRUs(); const targetConnection = savedUriList.find((uriEntry) => uriEntry.serverId === kernelConnection.serverId); // We only show this if we have a display name and the name is not the same as the URI (this prevents showing the long token for user entered URIs). diff --git a/src/notebooks/controllers/controllerRegistration.ts b/src/notebooks/controllers/controllerRegistration.ts index 77170a30163..c17f9763b62 100644 --- a/src/notebooks/controllers/controllerRegistration.ts +++ b/src/notebooks/controllers/controllerRegistration.ts @@ -103,9 +103,9 @@ export class ControllerRegistration implements IControllerRegistration, IExtensi this.disposables ); this.pythonEnvFilter.onDidChange(this.onDidChangeFilter, this, this.disposables); - this.serverUriStorage.onDidChangeUri(this.onDidChangeFilter, this, this.disposables); - this.serverUriStorage.onDidChangeUri(this.onDidChangeUri, this, this.disposables); - this.serverUriStorage.onDidRemoveUris(this.onDidRemoveUris, this, this.disposables); + this.serverUriStorage.onDidChange(this.onDidChangeFilter, this, this.disposables); + this.serverUriStorage.onDidChange(this.onDidChangeUri, this, this.disposables); + this.serverUriStorage.onDidRemove(this.onDidRemoveUris, this, this.disposables); this.onDidChange( ({ added }) => { diff --git a/src/notebooks/controllers/controllerRegistration.unit.test.ts b/src/notebooks/controllers/controllerRegistration.unit.test.ts index dfc41f3e44d..42ccc343e88 100644 --- a/src/notebooks/controllers/controllerRegistration.unit.test.ts +++ b/src/notebooks/controllers/controllerRegistration.unit.test.ts @@ -192,8 +192,8 @@ suite('Controller Registration', () => { when(kernelFinder.onDidChangeKernels).thenReturn(onDidChangeKernels.event); when(kernelFinder.onDidChangeRegistrations).thenReturn(onDidChangeRegistrations.event); when(kernelFilter.onDidChange).thenReturn(onDidChangeFilter.event); - when(serverUriStorage.onDidChangeUri).thenReturn(onDidChangeUri.event); - when(serverUriStorage.onDidRemoveUris).thenReturn(onDidRemoveUris.event); + when(serverUriStorage.onDidChange).thenReturn(onDidChangeUri.event); + when(serverUriStorage.onDidRemove).thenReturn(onDidRemoveUris.event); when(interpreters.onDidChangeInterpreter).thenReturn(onDidChangeInterpreter.event); when(interpreters.onDidChangeInterpreters).thenReturn(onDidChangeInterpreters.event); when(contributedLocalKernelFinder.onDidChangeKernels).thenReturn( diff --git a/src/notebooks/controllers/kernelSource/notebookKernelSourceSelector.ts b/src/notebooks/controllers/kernelSource/notebookKernelSourceSelector.ts index e940f8bacab..e1d628e9e35 100644 --- a/src/notebooks/controllers/kernelSource/notebookKernelSourceSelector.ts +++ b/src/notebooks/controllers/kernelSource/notebookKernelSourceSelector.ts @@ -181,7 +181,7 @@ export class NotebookKernelSourceSelector implements INotebookKernelSourceSelect multiStep: IMultiStepInput, state: MultiStepResult ): Promise | void> { - const savedURIList = await this.serverUriStorage.getSavedUriList(); + const savedURIList = await this.serverUriStorage.getMRUs(); if (token.isCancellationRequested) { return; @@ -335,7 +335,7 @@ export class NotebookKernelSourceSelector implements INotebookKernelSourceSelect if (token.isCancellationRequested) { throw new CancellationError(); } - await this.serverSelector.setJupyterURIToRemote(uri); + await this.serverSelector.addJupyterServer(uri); if (token.isCancellationRequested) { throw new CancellationError(); } diff --git a/src/notebooks/controllers/remoteKernelControllerWatcher.ts b/src/notebooks/controllers/remoteKernelControllerWatcher.ts index 4bd7fc09a9b..f0566abe1f6 100644 --- a/src/notebooks/controllers/remoteKernelControllerWatcher.ts +++ b/src/notebooks/controllers/remoteKernelControllerWatcher.ts @@ -47,7 +47,7 @@ export class RemoteKernelControllerWatcher implements IExtensionSyncActivationSe if (!provider.getHandles) { return; } - const [handles, uris] = await Promise.all([provider.getHandles(), this.uriStorage.getSavedUriList()]); + const [handles, uris] = await Promise.all([provider.getHandles(), this.uriStorage.getMRUs()]); const serverJupyterProviderMap = new Map(); const registeredHandles: string[] = []; await Promise.all( @@ -71,9 +71,9 @@ export class RemoteKernelControllerWatcher implements IExtensionSyncActivationSe // If not then remove this uri from the list. if (!handles.includes(info.handle)) { // Looks like the 3rd party provider has updated its handles and this server is no longer available. - await this.uriStorage.removeUri(item.uri); + await this.uriStorage.removeMRU(item.uri); } else if (!item.isValidated) { - await this.uriStorage.setUriToRemote(item.uri, item.displayName ?? item.uri).catch(noop); + await this.uriStorage.addMRU(item.uri, item.displayName ?? item.uri).catch(noop); } }) ); @@ -85,7 +85,7 @@ export class RemoteKernelControllerWatcher implements IExtensionSyncActivationSe try { const serverUri = await provider.getServerUri(handle); const uri = generateUriFromRemoteProvider(provider.id, handle); - await this.uriStorage.setUriToRemote(uri, serverUri.displayName); + await this.uriStorage.addMRU(uri, serverUri.displayName); } catch (ex) { traceError(`Failed to get server uri and add it to uri Storage for handle ${handle}`, ex); } diff --git a/src/notebooks/controllers/remoteKernelControllerWatcher.unit.test.ts b/src/notebooks/controllers/remoteKernelControllerWatcher.unit.test.ts index ba2b082308c..595edd7b63f 100644 --- a/src/notebooks/controllers/remoteKernelControllerWatcher.unit.test.ts +++ b/src/notebooks/controllers/remoteKernelControllerWatcher.unit.test.ts @@ -38,7 +38,7 @@ suite('RemoteKernelControllerWatcher', () => { onDidChangeProviders = new EventEmitter(); disposables.push(onDidChangeProviders); when(uriProviderRegistration.onDidChangeProviders).thenReturn(onDidChangeProviders.event); - when(uriStorage.removeUri(anything())).thenResolve(); + when(uriStorage.removeMRU(anything())).thenResolve(); watcher = new RemoteKernelControllerWatcher( disposables, instance(uriProviderRegistration), @@ -117,10 +117,10 @@ suite('RemoteKernelControllerWatcher', () => { instance(remoteLiveKernel) ]); - when(uriStorage.getSavedUriList()).thenResolve([ + when(uriStorage.getMRUs()).thenResolve([ { time: 1, serverId, uri: remoteUriForProvider1, displayName: 'Something' } ]); - when(uriStorage.setUriToRemote(anything(), anything())).thenResolve(); + when(uriStorage.addMRU(anything(), anything())).thenResolve(); watcher.activate(); @@ -152,7 +152,7 @@ suite('RemoteKernelControllerWatcher', () => { await onDidChangeHandles!(); assert.isOk(onDidChangeHandles, 'onDidChangeHandles should be defined'); - verify(uriStorage.removeUri(anything())).never(); + verify(uriStorage.removeMRU(anything())).never(); verify(localKernel.dispose()).never(); verify(remoteKernelSpec.dispose()).never(); verify(remoteLiveKernel.dispose()).never(); @@ -163,7 +163,7 @@ suite('RemoteKernelControllerWatcher', () => { await onDidChangeHandles!(); assert.isOk(onDidChangeHandles, 'onDidChangeHandles should be defined'); - verify(uriStorage.removeUri(remoteUriForProvider1)).once(); + verify(uriStorage.removeMRU(remoteUriForProvider1)).once(); verify(localKernel.dispose()).never(); verify(remoteKernelSpec.dispose()).once(); verify(remoteLiveKernel.dispose()).once(); diff --git a/src/platform/common/constants.ts b/src/platform/common/constants.ts index 9e81cd45474..b4cf15127ce 100644 --- a/src/platform/common/constants.ts +++ b/src/platform/common/constants.ts @@ -411,6 +411,7 @@ export enum Telemetry { FailedToUpdateKernelSpec = 'DS_INTERNAL.FAILED_TO_UPDATE_JUPYTER_KERNEL_SPEC', CellOutputMimeType = 'DS_INTERNAL.CELL_OUTPUT_MIME_TYPE', JupyterApiUsage = 'DATASCIENCE.JUPYTER_API_USAGE', + JupyterServerProviderResponseApi = 'DATASCIENCE.JUPYTER_SERVER_PROVIDER_RESPONSE_API', JupyterKernelApiUsage = 'DATASCIENCE.JUPYTER_KERNEL_API_USAGE', JupyterKernelApiAccess = 'DATASCIENCE.JUPYTER_KERNEL_API_ACCESS', JupyterKernelSpecEnumeration = 'DATASCIENCE.JUPYTER_KERNEL_SPEC_FETCH_FAILURE', diff --git a/src/standalone/api/api.ts b/src/standalone/api/api.ts index c72ccbd0d40..2d051a9a6f1 100644 --- a/src/standalone/api/api.ts +++ b/src/standalone/api/api.ts @@ -170,7 +170,7 @@ export function buildApi( controllerRegistration ); - await selector.setJupyterURIToRemote(uri); + await selector.addJupyterServer(uri); await controllerCreatedPromise; resolve(); }); diff --git a/src/standalone/api/extension.d.ts b/src/standalone/api/extension.d.ts index a3b4b36a4ba..91025c555b3 100644 --- a/src/standalone/api/extension.d.ts +++ b/src/standalone/api/extension.d.ts @@ -49,10 +49,6 @@ export interface IJupyterServerUri { * Authorization header to be used when connecting to the server. */ authorizationHeader: Record; - /** - * Date/time when header expires and should be refreshed. - */ - expiration?: Date; displayName: string; /** * The local directory that maps to the remote directory of the Jupyter Server. diff --git a/src/standalone/userJupyterServer/userServerUrlProvider.ts b/src/standalone/userJupyterServer/userServerUrlProvider.ts index 0363fad89a9..6a5024d7aaf 100644 --- a/src/standalone/userJupyterServer/userServerUrlProvider.ts +++ b/src/standalone/userJupyterServer/userServerUrlProvider.ts @@ -88,7 +88,7 @@ export class UserJupyterServerUrlProvider implements IExtensionSyncActivationSer this._initializeCachedServerInfo() .then(async () => { // once cache is initialized, check if we should do migration - const existingServers = await this.serverUriStorage.getSavedUriList(); + const existingServers = await this.serverUriStorage.getMRUs(); const migratedServers = []; for (const server of existingServers) { const info = extractJupyterServerHandleAndId(server.uri); diff --git a/src/telemetry.ts b/src/telemetry.ts index 9000b18e92d..70f82c21a29 100644 --- a/src/telemetry.ts +++ b/src/telemetry.ts @@ -21,6 +21,7 @@ import { PreferredKernelExactMatchReason } from './notebooks/controllers/types'; import { ExcludeType, PickType } from './platform/common/utils/misc'; import { SharedPropertyMapping } from './platform/telemetry/index'; import { IExtensionApi } from './standalone/api/api'; +import { IJupyterServerUri } from './kernels/jupyter/types'; export * from './platform/telemetry/index'; export type DurationMeasurement = { @@ -2038,6 +2039,10 @@ export class IEventNamePropertyMapping { * Error message when fallback module fails to load. */ fallbackErrorMessage?: string; + /** + * List of binaries found on disc. + */ + zmqBinaries?: string[]; }> = { owner: 'donjayamanne', feature: 'N/A', @@ -2082,6 +2087,10 @@ export class IEventNamePropertyMapping { fallbackErrorMessage: { classification: 'CallstackOrException', purpose: 'FeatureInsight' + }, + zmqBinaries: { + classification: 'CallstackOrException', + purpose: 'FeatureInsight' } } }; @@ -3560,6 +3569,38 @@ export class IEventNamePropertyMapping { } } }; + /** + * Telemetry sent when an Jupyter Server is returned by 3rd party extension. + */ + [Telemetry.JupyterServerProviderResponseApi]: TelemetryEventInfo<{ + providerId: string; + /** + * Extension Id that's attempting to use the API. + */ + extensionId: string; + /** + * Name of the API member used. + */ + pemUsed: (keyof IJupyterServerUri)[]; + }> = { + owner: 'donjayamanne', + feature: 'N/A', + source: 'N/A', + properties: { + providerId: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' + }, + extensionId: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' + }, + pemUsed: { + classification: 'PublicNonPersonalData', + purpose: 'FeatureInsight' + } + } + }; /** * Telemetry sent when an extension attempts to use our 3rd party API. */ diff --git a/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/serverPicker.ts b/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/serverPicker.ts index 357ac075573..cf4f4148009 100644 --- a/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/serverPicker.ts +++ b/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/serverPicker.ts @@ -76,18 +76,10 @@ export class RemoteServerPickerExample implements IJupyterUriProvider { // some other stuff if (stdout) { const output = JSON.parse(stdout.toString()); - const currentDate = new Date(); resolve({ baseUrl: Compute_ServerUri, token: '', //output.accessToken, - authorizationHeader: { Authorization: `Bearer ${output.accessToken}` }, - expiration: new Date( - currentDate.getFullYear(), - currentDate.getMonth(), - undefined, - currentDate.getHours(), - currentDate.getMinutes() + 1 // Expire after one minute - ) + authorizationHeader: { Authorization: `Bearer ${output.accessToken}` } }); } else { reject('Unable to get az token'); diff --git a/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/typings/jupyter.d.ts b/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/typings/jupyter.d.ts index e5e4bab171d..e31373be1c5 100644 --- a/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/typings/jupyter.d.ts +++ b/src/test/datascience/extensionapi/exampleextension/ms-ai-tools-test/src/typings/jupyter.d.ts @@ -52,7 +52,6 @@ export interface IJupyterServerUri { token: string; // tslint:disable-next-line: no-any authorizationHeader: any; // JSON object for authorization header. - expiration?: Date; // Date/time when header expires and should be refreshed. } export type JupyterServerUriHandle = string; diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index 74c26023152..2788e76b224 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -347,7 +347,7 @@ async function shutdownRemoteKernels() { const cancelToken = new CancellationTokenSource(); let sessionManager: IJupyterSessionManager | undefined; try { - const uris = await serverUriStorage.getSavedUriList(); + const uris = await serverUriStorage.getMRUs(); const connection = await jupyterConnection.createConnectionInfo({ serverId: uris[0].serverId }); diff --git a/src/test/datascience/notebook/kernelSelection.vscode.test.ts b/src/test/datascience/notebook/kernelSelection.vscode.test.ts index b46ef481b50..1e1f01c9095 100644 --- a/src/test/datascience/notebook/kernelSelection.vscode.test.ts +++ b/src/test/datascience/notebook/kernelSelection.vscode.test.ts @@ -175,7 +175,7 @@ suite('Kernel Selection @kernelPicker', function () { await closeNotebooksAndCleanUpAfterTests(disposables); console.log(`End test completed ${this.currentTest?.title}`); if (jupyterServerUri) { - await serverUriStorage.setUriToRemote(jupyterServerUri, ''); + await serverUriStorage.addMRU(jupyterServerUri, ''); } }); diff --git a/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts b/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts index 0fe811fff71..cb079d688c3 100644 --- a/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts +++ b/src/test/datascience/notebook/remoteNotebookEditor.vscode.test.ts @@ -128,7 +128,7 @@ suite('Remote Kernel Execution', function () { const uri = await JupyterServer.instance.startSecondJupyterWithToken(); const uriString = decodeURIComponent(uri.toString()); traceInfo(`Another Jupyter started and listening at ${uriString}`); - await jupyterServerSelector.setJupyterURIToRemote(uriString); + await jupyterServerSelector.addJupyterServer(uriString); // Opening a notebook will trigger the refresh of the kernel list. nbUri = await createTemporaryNotebook([], disposables); From a89e0b62667ab6f345cda8b8104101277197e37d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 18 May 2023 15:13:04 +1000 Subject: [PATCH 2/2] rename properties --- src/kernels/jupyter/jupyterUtils.ts | 12 +++++++----- src/kernels/jupyter/session/jupyterSessionManager.ts | 2 +- src/kernels/types.ts | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/kernels/jupyter/jupyterUtils.ts b/src/kernels/jupyter/jupyterUtils.ts index 955f33bede1..122c11dce4c 100644 --- a/src/kernels/jupyter/jupyterUtils.ts +++ b/src/kernels/jupyter/jupyterUtils.ts @@ -92,7 +92,7 @@ export async function handleExpiredCertsError( export function createRemoteConnectionInfo( uri: string, serverUri?: IJupyterServerUri, - serverId?: string + providerId?: string ): IJupyterConnection { let url: URL; try { @@ -102,7 +102,7 @@ export function createRemoteConnectionInfo( throw err; } - serverId = serverId || ''; + providerId = providerId || ''; const baseUrl = serverUri ? serverUri.baseUrl @@ -115,7 +115,7 @@ export function createRemoteConnectionInfo( return { type: 'jupyter', baseUrl, - serverId, + providerId, token, hostName, localLaunch: false, @@ -134,11 +134,13 @@ export function createRemoteConnectionInfo( // For remote jupyter servers that are managed by us, we can provide the auth header. // Its crucial this is set to undefined, else password retrieval will not be attempted. getAuthHeader: - serverUri && !serverId.startsWith('_builtin') && !isEmptyAuthHeader + serverUri && !providerId.startsWith('_builtin') && !isEmptyAuthHeader ? () => serverUri?.authorizationHeader : undefined, getWebsocketProtocols: - serverUri && !serverId.startsWith('_builtin') ? () => serverUri?.webSocketProtocols || [] : () => [], + serverUri && !providerId.startsWith('_builtin') && (serverUri?.webSocketProtocols || []).length + ? () => serverUri?.webSocketProtocols || [] + : () => [], url: uri }; } diff --git a/src/kernels/jupyter/session/jupyterSessionManager.ts b/src/kernels/jupyter/session/jupyterSessionManager.ts index 493dd994f19..eaba4c2ed2c 100644 --- a/src/kernels/jupyter/session/jupyterSessionManager.ts +++ b/src/kernels/jupyter/session/jupyterSessionManager.ts @@ -456,7 +456,7 @@ export class JupyterSessionManager implements IJupyterSessionManager { let serverSecurePromise = JupyterSessionManager.secureServers.get(connInfo.baseUrl); if (serverSecurePromise === undefined) { - if (connInfo.serverId && !connInfo.serverId.startsWith('_builtin')) { + if (connInfo.providerId && !connInfo.providerId.startsWith('_builtin')) { // If a Jupyter URI provider is providing this URI, then we trust it. serverSecurePromise = Promise.resolve(true); JupyterSessionManager.secureServers.set(connInfo.baseUrl, serverSecurePromise); diff --git a/src/kernels/types.ts b/src/kernels/types.ts index dbddb9342db..c27c6fa7e3b 100644 --- a/src/kernels/types.ts +++ b/src/kernels/types.ts @@ -536,7 +536,7 @@ export interface IJupyterConnection extends Disposable { // Jupyter specific members readonly baseUrl: string; readonly token: string; - readonly serverId?: string; + readonly providerId?: string; readonly hostName: string; readonly rootDirectory: Uri; // Directory where the notebook server was started. readonly url: string;