From 2b3f5d5182d218c1c06d2aaff2ea3316a3d6af5a Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 2 Nov 2022 18:18:09 -0700 Subject: [PATCH 1/4] Merge remote kernel finders. --- .../jupyter/finder/remoteKernelFinder.ts | 426 ------------------ .../finder/remoteKernelFinder.unit.test.ts | 15 +- .../finder/universalRemoteKernelFinder.ts | 21 +- .../universalRemoteKernelFinderController.ts | 184 +++++++- .../jupyter/launcher/serverUriStorage.ts | 3 + src/kernels/jupyter/serviceRegistry.node.ts | 18 +- src/kernels/jupyter/serviceRegistry.web.ts | 23 +- src/kernels/jupyter/types.ts | 2 + .../raw/finder/localKernelFinder.unit.test.ts | 6 +- 9 files changed, 198 insertions(+), 500 deletions(-) delete mode 100644 src/kernels/jupyter/finder/remoteKernelFinder.ts diff --git a/src/kernels/jupyter/finder/remoteKernelFinder.ts b/src/kernels/jupyter/finder/remoteKernelFinder.ts deleted file mode 100644 index 925ab091715..00000000000 --- a/src/kernels/jupyter/finder/remoteKernelFinder.ts +++ /dev/null @@ -1,426 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -'use strict'; - -import { injectable, inject, named } from 'inversify'; -import { CancellationToken, CancellationTokenSource, Event, EventEmitter, Memento, Uri } from 'vscode'; -import { getKernelId, getLanguageInKernelSpec, serializeKernelConnection } from '../../helpers'; -import { - IJupyterKernelSpec, - IKernelFinder, - IKernelProvider, - INotebookProvider, - INotebookProviderConnection, - isRemoteConnection, - LiveRemoteKernelConnectionMetadata, - RemoteKernelConnectionMetadata, - RemoteKernelSpecConnectionMetadata -} from '../../types'; -import { - GLOBAL_MEMENTO, - IDisposableRegistry, - IExtensions, - IMemento, - IsWebExtension -} from '../../../platform/common/types'; -import { IInterpreterService } from '../../../platform/interpreter/contracts'; -import { capturePerfTelemetry, Telemetry } from '../../../telemetry'; -import { - IJupyterSessionManagerFactory, - IJupyterSessionManager, - IJupyterServerUriStorage, - IServerConnectionType, - IJupyterRemoteCachedKernelValidator, - IRemoteKernelFinder -} from '../types'; -import { sendKernelSpecTelemetry } from '../../raw/finder/helper'; -import { traceError, traceInfoIfCI, traceVerbose, traceWarning } from '../../../platform/logging'; -import { IPythonExtensionChecker } from '../../../platform/api/types'; -import { computeServerId } from '../jupyterUtils'; -import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; -import { createPromiseFromCancellation } from '../../../platform/common/cancellation'; -import { DisplayOptions } from '../../displayOptions'; -import { isArray } from '../../../platform/common/utils/sysTypes'; -import { deserializeKernelConnection } from '../../helpers'; -import * as localize from '../../../platform/common/utils/localize'; -import { noop } from '../../../platform/common/utils/misc'; -import { IApplicationEnvironment } from '../../../platform/common/application/types'; -import { KernelFinder } from '../../kernelFinder'; -import { RemoteKernelSpecsCacheKey, removeOldCachedItems } from '../../common/commonFinder'; -import { IExtensionSingleActivationService } from '../../../platform/activation/types'; -import { ContributedKernelFinderKind } from '../../internalTypes'; - -// Even after shutting down a kernel, the server API still returns the old information. -// Re-query after 2 seconds to ensure we don't get stale information. -const REMOTE_KERNEL_REFRESH_INTERVAL = 2_000; - -// This class finds kernels from the current selected Jupyter URI -@injectable() -export class RemoteKernelFinder implements IRemoteKernelFinder, IExtensionSingleActivationService { - /** - * List of ids of kernels that should be hidden from the kernel picker. - */ - private readonly kernelIdsToHide = new Set(); - kind = ContributedKernelFinderKind.Remote; - id: string = 'currentremote'; - displayName: string = localize.DataScience.remoteKernelFinderDisplayName(); - private _cacheUpdateCancelTokenSource: CancellationTokenSource | undefined; - private cache: RemoteKernelConnectionMetadata[] = []; - - private _onDidChangeKernels = new EventEmitter(); - onDidChangeKernels: Event = this._onDidChangeKernels.event; - - private wasPythonInstalledWhenFetchingKernels = false; - - constructor( - @inject(IJupyterSessionManagerFactory) private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, - @inject(IInterpreterService) private interpreterService: IInterpreterService, - @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker, - @inject(INotebookProvider) private readonly notebookProvider: INotebookProvider, - @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage, - @inject(IServerConnectionType) private readonly serverConnectionType: IServerConnectionType, - @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento, - @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, - @inject(IJupyterRemoteCachedKernelValidator) - private readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, - @inject(IKernelFinder) kernelFinder: KernelFinder, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, - @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, - @inject(IExtensions) private readonly extensions: IExtensions, - @inject(IsWebExtension) private isWebExtension: boolean - ) { - kernelFinder.registerKernelFinder(this); - } - - async activate(): Promise { - // warm up the cache - this.loadCache().then(noop, noop); - - this.disposables.push( - this.serverUriStorage.onDidChangeUri(() => { - this.updateCache().then(noop, noop); - }) - ); - - // If we create a new kernel, we need to refresh if the kernel is remote (because - // we have live sessions possible) - // Note, this is a perf optimization for right now. We should not need - // to check for remote if the future when we support live sessions on local - this.kernelProvider.onDidStartKernel( - (k) => { - if (isRemoteConnection(k.kernelConnectionMetadata)) { - // update remote kernels - this.updateCache().then(noop, noop); - } - }, - this, - this.disposables - ); - - // For kernel dispose we need to wait a bit, otherwise the list comes back the - // same - this.kernelProvider.onDidDisposeKernel( - (k) => { - if (k && isRemoteConnection(k.kernelConnectionMetadata)) { - const timer = setTimeout(() => { - this.updateCache().then(noop, noop); - }, REMOTE_KERNEL_REFRESH_INTERVAL); - - return timer; - } - }, - this, - this.disposables - ); - - this.extensions.onDidChange( - () => { - // If we just installed the Python extension and we fetched the controllers, then fetch it again. - if (!this.wasPythonInstalledWhenFetchingKernels && this.extensionChecker.isPythonExtensionInstalled) { - this.updateCache().then(noop, noop); - } - }, - this, - this.disposables - ); - this.wasPythonInstalledWhenFetchingKernels = this.extensionChecker.isPythonExtensionInstalled; - } - - public async loadCache() { - traceVerbose('RemoteKernelFinder: load cache', this.serverConnectionType.isLocalLaunch); - - if (this.serverConnectionType.isLocalLaunch) { - await this.writeToCache([]); - return; - } - - const uri = await this.serverUriStorage.getRemoteUri(); - if (!uri || !uri.isValidated) { - await this.writeToCache([]); - return; - } - - const kernelsFromCache = await this.getFromCache(); - - let kernels: RemoteKernelConnectionMetadata[] = []; - - // If we finish the cache first, and we don't have any items, in the cache, then load without cache. - if (Array.isArray(kernelsFromCache) && kernelsFromCache.length > 0) { - kernels = kernelsFromCache; - // kick off a cache update request - this.updateCache().then(noop, noop); - } else { - try { - const kernelsWithoutCachePromise = (async () => { - const connInfo = await this.getRemoteConnectionInfo(); - return connInfo ? this.listKernelsFromConnection(connInfo) : Promise.resolve([]); - })(); - - kernels = await kernelsWithoutCachePromise; - } catch (ex) { - traceError('RemoteKernelFinder: Failed to get kernels without cache', ex); - } - } - - await this.writeToCache(kernels); - - traceVerbose('RemoteKernelFinder: load cache finished'); - } - - private async updateCache() { - let kernels: RemoteKernelConnectionMetadata[] = []; - this._cacheUpdateCancelTokenSource?.dispose(); - const updateCacheCancellationToken = new CancellationTokenSource(); - this._cacheUpdateCancelTokenSource = updateCacheCancellationToken; - - try { - const kernelsWithoutCachePromise = (async () => { - const connInfo = await this.getRemoteConnectionInfo(updateCacheCancellationToken.token); - return connInfo ? this.listKernelsFromConnection(connInfo) : Promise.resolve([]); - })(); - - kernels = await kernelsWithoutCachePromise; - } catch (ex) { - traceWarning(`Could not fetch kernels from the ${this.kind} server, falling back to cache: ${ex}`); - // Since fetching the remote kernels failed, we fall back to the cache, - // at this point no need to display all of the kernel specs, - // Its possible the connection is dead, just display the live kernels we had. - // I.e. if user had a notebook connected to a remote kernel, then just display that live kernel. - kernels = await this.getFromCache(updateCacheCancellationToken.token); - kernels = kernels.filter((item) => item.kind === 'connectToLiveRemoteKernel'); - } - - if (updateCacheCancellationToken.token.isCancellationRequested) { - return; - } - - await this.writeToCache(kernels); - - this._onDidChangeKernels.fire(); - } - - /** - * - * Remote kernel finder is resource agnostic. - */ - public get kernels(): RemoteKernelConnectionMetadata[] { - return this.cache; - } - - private async getRemoteConnectionInfo( - cancelToken?: CancellationToken - ): Promise { - const ui = new DisplayOptions(false); - const uri = await this.serverUriStorage.getRemoteUri(); - if (!uri || !uri.isValidated) { - return; - } - return this.notebookProvider.connect({ - resource: undefined, - ui, - localJupyter: false, - token: cancelToken, - serverId: this.serverUriStorage.currentServerId! - }); - } - - private async getFromCache(cancelToken?: CancellationToken): Promise { - try { - traceVerbose('RemoteKernelFinder: get from cache'); - - let results: RemoteKernelConnectionMetadata[] = this.cache; - const key = this.getCacheKey(); - - // If not in memory, check memento - if (!results || results.length === 0) { - // Check memento too - const values = this.globalState.get<{ - kernels: RemoteKernelConnectionMetadata[]; - extensionVersion: string; - }>(key, { kernels: [], extensionVersion: '' }); - - if (values && isArray(values.kernels) && values.extensionVersion === this.env.extensionVersion) { - results = values.kernels.map(deserializeKernelConnection) as RemoteKernelConnectionMetadata[]; - this.cache = results; - } - } - - // Validate - const validValues: RemoteKernelConnectionMetadata[] = []; - const promise = Promise.all( - results.map(async (item) => { - if (await this.isValidCachedKernel(item)) { - validValues.push(item); - } - }) - ); - if (cancelToken) { - await Promise.race([ - promise, - createPromiseFromCancellation({ - token: cancelToken, - cancelAction: 'resolve', - defaultValue: undefined - }) - ]); - } else { - await promise; - } - return validValues; - } catch (ex) { - traceError('RemoteKernelFinder: Failed to get from cache', ex); - } - - return []; - } - - // Talk to the remote server to determine sessions - @capturePerfTelemetry(Telemetry.KernelListingPerf, { kind: 'remote' }) - public async listKernelsFromConnection( - connInfo: INotebookProviderConnection - ): Promise { - // Get a jupyter session manager to talk to - let sessionManager: IJupyterSessionManager | undefined; - // This should only be used when doing remote. - if (connInfo.type === 'jupyter') { - try { - sessionManager = await this.jupyterSessionManagerFactory.create(connInfo); - - // Get running and specs at the same time - const [running, specs, sessions, serverId] = await Promise.all([ - sessionManager.getRunningKernels(), - sessionManager.getKernelSpecs(), - sessionManager.getRunningSessions(), - computeServerId(connInfo.url) - ]); - - // Turn them both into a combined list - const mappedSpecs = await Promise.all( - specs.map(async (s) => { - sendKernelSpecTelemetry(s, 'remote'); - const kernel: RemoteKernelSpecConnectionMetadata = { - kind: 'startUsingRemoteKernelSpec', - interpreter: await this.getInterpreter(s, connInfo.baseUrl), - kernelSpec: s, - id: getKernelId(s, undefined, serverId), - baseUrl: connInfo.baseUrl, - serverId: serverId - }; - return kernel; - }) - ); - const mappedLive = sessions.map((s) => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const liveKernel = s.kernel as any; - const lastActivityTime = liveKernel.last_activity - ? new Date(Date.parse(liveKernel.last_activity.toString())) - : new Date(); - const numberOfConnections = liveKernel.connections - ? parseInt(liveKernel.connections.toString(), 10) - : 0; - const activeKernel = running.find((active) => active.id === s.kernel?.id) || {}; - const matchingSpec: Partial = - specs.find((spec) => spec.name === s.kernel?.name) || {}; - - const kernel: LiveRemoteKernelConnectionMetadata = { - kind: 'connectToLiveRemoteKernel', - kernelModel: { - ...s.kernel, - ...matchingSpec, - ...activeKernel, - name: s.kernel?.name || '', - lastActivityTime, - numberOfConnections, - model: s - }, - baseUrl: connInfo.baseUrl, - id: s.kernel?.id || '', - serverId - }; - return kernel; - }); - - // Filter out excluded ids - const filtered = mappedLive.filter((k) => !this.kernelIdsToHide.has(k.kernelModel.id || '')); - const items = [...filtered, ...mappedSpecs]; - return items; - } catch (ex) { - traceError(`Error fetching remote kernels:`, ex); - throw ex; - } finally { - if (sessionManager) { - await sessionManager.dispose(); - } - } - } - return []; - } - - private async getInterpreter(spec: IJupyterKernelSpec, baseUrl: string) { - const parsed = new URL(baseUrl); - if ( - (parsed.hostname.toLocaleLowerCase() === 'localhost' || parsed.hostname === '127.0.0.1') && - this.extensionChecker.isPythonExtensionInstalled && - !this.isWebExtension && - getLanguageInKernelSpec(spec) === PYTHON_LANGUAGE - ) { - // Interpreter is possible. Same machine as VS code - try { - traceInfoIfCI(`Getting interpreter details for localhost remote kernel: ${spec.name}`); - return await this.interpreterService.getInterpreterDetails(Uri.file(spec.argv[0])); - } catch (ex) { - traceError(`Failure getting interpreter details for remote kernel: `, ex); - } - } - } - - private getCacheKey() { - return RemoteKernelSpecsCacheKey; - } - - private async writeToCache(values: RemoteKernelConnectionMetadata[]) { - try { - traceVerbose(`RemoteKernelFinder: Writing ${values.length} remote kernel connection metadata to cache`); - const key = this.getCacheKey(); - this.cache = values; - const serialized = values.map(serializeKernelConnection); - await Promise.all([ - removeOldCachedItems(this.globalState), - this.globalState.update(key, { kernels: serialized, extensionVersion: this.env.extensionVersion }) - ]); - } catch (ex) { - traceError('RemoteKernelFinder: Failed to write to cache', ex); - } - } - - private async isValidCachedKernel(kernel: RemoteKernelConnectionMetadata): Promise { - switch (kernel.kind) { - case 'startUsingRemoteKernelSpec': - // Always fetch the latest kernels from remotes, no need to display cached remote kernels. - return false; - case 'connectToLiveRemoteKernel': - return this.cachedRemoteKernelValidator.isValid(kernel); - } - } -} diff --git a/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts b/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts index 48bc2af9453..06250429bd9 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts @@ -23,7 +23,6 @@ import { import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { JupyterSessionManager } from '../session/jupyterSessionManager'; import { JupyterSessionManagerFactory } from '../session/jupyterSessionManagerFactory'; -import { RemoteKernelFinder } from './remoteKernelFinder'; import { ILocalKernelFinder } from '../../raw/types'; import { ActiveKernelIdList, PreferredRemoteKernelIdProvider } from '../preferredRemoteKernelIdProvider'; import { @@ -46,11 +45,12 @@ import { KernelRankingHelper } from '../../../notebooks/controllers/kernelRankin import { IExtensions } from '../../../platform/common/types'; import { takeTopRankKernel } from '../../raw/finder/localKernelFinder.unit.test'; import { createEventHandler, TestEventHandler } from '../../../test/common'; +import { UniversalRemoteKernelFinder } from './universalRemoteKernelFinder'; suite(`Remote Kernel Finder`, () => { let disposables: Disposable[] = []; let preferredRemoteKernelIdProvider: PreferredRemoteKernelIdProvider; - let remoteKernelFinder: RemoteKernelFinder; + let remoteKernelFinder: UniversalRemoteKernelFinder; let localKernelFinder: ILocalKernelFinder; let kernelFinder: KernelFinder; let kernelRankHelper: IKernelRankingHelper; @@ -175,21 +175,22 @@ suite(`Remote Kernel Finder`, () => { disposables.push(kernelsChanged); kernelRankHelper = new KernelRankingHelper(preferredRemoteKernelIdProvider); - remoteKernelFinder = new RemoteKernelFinder( + remoteKernelFinder = new UniversalRemoteKernelFinder( + 'currentremote', + 'Local Kernels', + RemoteKernelSpecsCacheKey, instance(jupyterSessionManagerFactory), instance(interpreterService), instance(extensionChecker), instance(notebookProvider), - instance(serverUriStorage), - instance(connectionType), instance(memento), instance(env), instance(cachedRemoteKernelValidator), kernelFinder, - [], instance(kernelProvider), instance(extensions), - false + false, + serverEntry ); remoteKernelFinder.activate().then(noop, noop); }); diff --git a/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts b/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts index db6b3962e2e..9dd831b0933 100644 --- a/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts +++ b/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts @@ -32,13 +32,12 @@ import { computeServerId } from '../jupyterUtils'; import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; import { createPromiseFromCancellation } from '../../../platform/common/cancellation'; import { DisplayOptions } from '../../displayOptions'; -import * as localize from '../../../platform/common/utils/localize'; import { isArray } from '../../../platform/common/utils/sysTypes'; import { deserializeKernelConnection } from '../../helpers'; import { noop } from '../../../platform/common/utils/misc'; import { IApplicationEnvironment } from '../../../platform/common/application/types'; import { KernelFinder } from '../../kernelFinder'; -import { RemoteKernelSpecsCacheKey, removeOldCachedItems } from '../../common/commonFinder'; +import { removeOldCachedItems } from '../../common/commonFinder'; import { ContributedKernelFinderKind, IContributedKernelFinderInfo } from '../../internalTypes'; import { disposeAllDisposables } from '../../../platform/common/helpers'; @@ -53,8 +52,6 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri */ private readonly kernelIdsToHide = new Set(); kind = ContributedKernelFinderKind.Remote; - id: string; - displayName: string; private _cacheUpdateCancelTokenSource: CancellationTokenSource | undefined; private cache: RemoteKernelConnectionMetadata[] = []; @@ -69,6 +66,9 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri private wasPythonInstalledWhenFetchingKernels = false; constructor( + readonly id: string /**`${this.kind}-${serverUri.serverId}` */, + readonly displayName: string /** localize.DataScience.universalRemoteKernelFinderDisplayName().format( serverUri.displayName || serverUri.uri ) */, + readonly cacheKey: string /** `${RemoteKernelSpecsCacheKey}-${this.serverUri.serverId}` */, private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, private interpreterService: IInterpreterService, private extensionChecker: IPythonExtensionChecker, @@ -82,14 +82,6 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri private isWebExtension: boolean, private readonly serverUri: IJupyterServerUriEntry ) { - // Register with remote-serverId as our ID - this.id = `${this.kind}-${serverUri.serverId}`; - - // Create a reasonable display name for this kernel finder - this.displayName = localize.DataScience.universalRemoteKernelFinderDisplayName().format( - serverUri.displayName || serverUri.uri - ); - // When we register, add a disposable to clean ourselves up from the main kernel finder list // Unlike the Local kernel finder universal remote kernel finders will be added on the fly this.disposables.push(kernelFinder.registerKernelFinder(this)); @@ -160,7 +152,7 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri this.wasPythonInstalledWhenFetchingKernels = this.extensionChecker.isPythonExtensionInstalled; } - private async loadCache() { + public async loadCache() { traceInfoIfCI(`Remote Kernel Finder load cache Server: ${this.id}`); const kernelsFromCache = await this.getFromCache(); @@ -390,8 +382,7 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri } private getCacheKey() { - // For Universal finders key each one per serverId - return `${RemoteKernelSpecsCacheKey}-${this.serverUri.serverId}`; + return this.cacheKey; } private async writeToCache(values: RemoteKernelConnectionMetadata[]) { diff --git a/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts b/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts index aca74a4b9c6..58c7b703786 100644 --- a/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts +++ b/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts @@ -8,6 +8,7 @@ import { Memento } from 'vscode'; import { IKernelFinder, IKernelProvider, INotebookProvider } from '../../types'; import { GLOBAL_MEMENTO, + IConfigurationService, IDisposableRegistry, IExtensions, IMemento, @@ -26,30 +27,35 @@ import { IApplicationEnvironment } from '../../../platform/common/application/ty import { KernelFinder } from '../../kernelFinder'; import { IExtensionSingleActivationService } from '../../../platform/activation/types'; import { UniversalRemoteKernelFinder } from './universalRemoteKernelFinder'; +import { ContributedKernelFinderKind } from '../../internalTypes'; +import * as localize from '../../../platform/common/utils/localize'; +import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder'; -// This class creates RemoteKernelFinders for all registered Jupyter Server URIs -@injectable() -export class UniversalRemoteKernelFinderController implements IExtensionSingleActivationService { +/** Strategy design */ +interface IRemoteKernelFinderRegistrationStrategy { + activate(): Promise; +} + +class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { private serverFinderMapping: Map = new Map< string, UniversalRemoteKernelFinder >(); constructor( - @inject(IJupyterSessionManagerFactory) private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, - @inject(IInterpreterService) private interpreterService: IInterpreterService, - @inject(IPythonExtensionChecker) private extensionChecker: IPythonExtensionChecker, - @inject(INotebookProvider) private readonly notebookProvider: INotebookProvider, - @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage, - @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento, - @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, - @inject(IJupyterRemoteCachedKernelValidator) + private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, + private interpreterService: IInterpreterService, + private extensionChecker: IPythonExtensionChecker, + private readonly notebookProvider: INotebookProvider, + private readonly serverUriStorage: IJupyterServerUriStorage, + private readonly globalState: Memento, + private readonly env: IApplicationEnvironment, private readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, - @inject(IKernelFinder) private readonly kernelFinder: KernelFinder, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, - @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, - @inject(IExtensions) private readonly extensions: IExtensions, - @inject(IsWebExtension) private isWebExtension: boolean + private readonly kernelFinder: KernelFinder, + private readonly disposables: IDisposableRegistry, + private readonly kernelProvider: IKernelProvider, + private readonly extensions: IExtensions, + private isWebExtension: boolean ) {} async activate(): Promise { @@ -66,11 +72,17 @@ export class UniversalRemoteKernelFinderController implements IExtensionSingleAc createRemoteKernelFinder(serverUri: IJupyterServerUriEntry) { if (!serverUri.isValidated) { + // TODO@rebornix, what if it's now validated? return; } if (!this.serverFinderMapping.has(serverUri.serverId)) { const finder = new UniversalRemoteKernelFinder( + `${ContributedKernelFinderKind.Remote}-${serverUri.serverId}`, + localize.DataScience.universalRemoteKernelFinderDisplayName().format( + serverUri.displayName || serverUri.uri + ), + `${RemoteKernelSpecsCacheKey}-${serverUri.serverId}`, this.jupyterSessionManagerFactory, this.interpreterService, this.extensionChecker, @@ -101,3 +113,143 @@ export class UniversalRemoteKernelFinderController implements IExtensionSingleAc }); } } + +class SingleServerStrategy implements IRemoteKernelFinderRegistrationStrategy { + private _activeServerFinder: { entry: IJupyterServerUriEntry; finder: UniversalRemoteKernelFinder } | undefined; + constructor( + private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, + private interpreterService: IInterpreterService, + private extensionChecker: IPythonExtensionChecker, + private readonly notebookProvider: INotebookProvider, + private readonly serverUriStorage: IJupyterServerUriStorage, + private readonly globalState: Memento, + private readonly env: IApplicationEnvironment, + private readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, + private readonly kernelFinder: KernelFinder, + private readonly disposables: IDisposableRegistry, + private readonly kernelProvider: IKernelProvider, + private readonly extensions: IExtensions, + private isWebExtension: boolean + ) {} + + async activate(): Promise { + this.disposables.push( + this.serverUriStorage.onDidChangeUri(() => { + this.updateRemoteKernelFinder().then(noop, noop); + }) + ); + + this.updateRemoteKernelFinder().then(noop, noop); + } + + async updateRemoteKernelFinder() { + if (this.serverUriStorage.isLocalLaunch) { + // no remote kernel finder needed + this._activeServerFinder?.finder.dispose(); + this._activeServerFinder = undefined; + return; + } + + const uri = await this.serverUriStorage.getRemoteUri(); + // uri should not be local + + if (!uri || !uri.isValidated) { + this._activeServerFinder?.finder.dispose(); + this._activeServerFinder = undefined; + return; + } + + if (this._activeServerFinder?.entry.serverId === uri.serverId) { + // no op + return; + } + + this._activeServerFinder?.finder.dispose(); + const finder = new UniversalRemoteKernelFinder( + 'currentremote', + localize.DataScience.remoteKernelFinderDisplayName(), + RemoteKernelSpecsCacheKey, + this.jupyterSessionManagerFactory, + this.interpreterService, + this.extensionChecker, + this.notebookProvider, + this.globalState, + this.env, + this.cachedRemoteKernelValidator, + this.kernelFinder, + this.kernelProvider, + this.extensions, + this.isWebExtension, + uri + ); + + this._activeServerFinder = { + entry: uri, + finder + }; + + finder.activate().then(noop, noop); + } +} + +// This class creates RemoteKernelFinders for all registered Jupyter Server URIs +@injectable() +export class UniversalRemoteKernelFinderController implements IExtensionSingleActivationService { + private _strategy: IRemoteKernelFinderRegistrationStrategy; + + constructor( + @inject(IConfigurationService) readonly configurationService: IConfigurationService, + @inject(IJupyterSessionManagerFactory) jupyterSessionManagerFactory: IJupyterSessionManagerFactory, + @inject(IInterpreterService) interpreterService: IInterpreterService, + @inject(IPythonExtensionChecker) extensionChecker: IPythonExtensionChecker, + @inject(INotebookProvider) notebookProvider: INotebookProvider, + @inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage, + @inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento, + @inject(IApplicationEnvironment) env: IApplicationEnvironment, + @inject(IJupyterRemoteCachedKernelValidator) + cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, + @inject(IKernelFinder) kernelFinder: KernelFinder, + @inject(IDisposableRegistry) disposables: IDisposableRegistry, + @inject(IKernelProvider) kernelProvider: IKernelProvider, + @inject(IExtensions) extensions: IExtensions, + @inject(IsWebExtension) isWebExtension: boolean + ) { + if (this.configurationService.getSettings().kernelPickerType === 'Insiders') { + this._strategy = new MultiServerStrategy( + jupyterSessionManagerFactory, + interpreterService, + extensionChecker, + notebookProvider, + serverUriStorage, + globalState, + env, + cachedRemoteKernelValidator, + kernelFinder, + disposables, + kernelProvider, + extensions, + isWebExtension + ); + } else { + this._strategy = new SingleServerStrategy( + jupyterSessionManagerFactory, + interpreterService, + extensionChecker, + notebookProvider, + serverUriStorage, + globalState, + env, + cachedRemoteKernelValidator, + kernelFinder, + disposables, + kernelProvider, + extensions, + isWebExtension + ); + } + } + + async activate(): Promise { + this._strategy.activate().then(noop, noop); + } +} diff --git a/src/kernels/jupyter/launcher/serverUriStorage.ts b/src/kernels/jupyter/launcher/serverUriStorage.ts index 4c1148bcf1b..3ae56b68ce9 100644 --- a/src/kernels/jupyter/launcher/serverUriStorage.ts +++ b/src/kernels/jupyter/launcher/serverUriStorage.ts @@ -56,6 +56,9 @@ export class JupyterServerUriStorage implements IJupyterServerUriStorage, IServe public get onDidChange(): Event { return this._onDidChangeUri.event; } + public get onDidChangeConnectionType(): Event { + return this._onDidChangeUri.event; + } public get isLocalLaunch(): boolean { return this._localOnly; } diff --git a/src/kernels/jupyter/serviceRegistry.node.ts b/src/kernels/jupyter/serviceRegistry.node.ts index 33a27705de9..2cdffd9570d 100644 --- a/src/kernels/jupyter/serviceRegistry.node.ts +++ b/src/kernels/jupyter/serviceRegistry.node.ts @@ -37,7 +37,6 @@ import { NotebookServerProvider } from './launcher/notebookServerProvider'; import { NotebookStarter } from './launcher/notebookStarter.node'; import { JupyterServerUriStorage } from './launcher/serverUriStorage'; import { LiveRemoteKernelConnectionUsageTracker } from './liveRemoteKernelConnectionTracker'; -import { RemoteKernelFinder } from './finder/remoteKernelFinder'; import { JupyterServerSelector } from './serverSelector'; import { BackingFileCreator } from './session/backingFileCreator.node'; import { JupyterRequestCreator } from './session/jupyterRequestCreator.node'; @@ -65,7 +64,6 @@ import { IServerConnectionType } from './types'; import { IJupyterCommandFactory, IJupyterSubCommandExecutionService } from './types.node'; -import { IConfigurationService } from '../../platform/common/types'; import { UniversalRemoteKernelFinderController } from './finder/universalRemoteKernelFinderController'; export function registerTypes(serviceManager: IServiceManager, _isDevMode: boolean) { @@ -153,16 +151,8 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole ); serviceManager.addSingleton(IExtensionSyncActivationService, JupyterDetectionTelemetry); serviceManager.addSingleton(IDataScienceErrorHandler, DataScienceErrorHandlerNode); - const configuration = serviceManager.get(IConfigurationService); - if (configuration.getSettings().kernelPickerType === 'Insiders') { - serviceManager.addSingleton( - IExtensionSingleActivationService, - UniversalRemoteKernelFinderController - ); - } else { - serviceManager.addSingleton( - IExtensionSingleActivationService, - RemoteKernelFinder - ); - } + serviceManager.addSingleton( + IExtensionSingleActivationService, + UniversalRemoteKernelFinderController + ); } diff --git a/src/kernels/jupyter/serviceRegistry.web.ts b/src/kernels/jupyter/serviceRegistry.web.ts index 5d2b87f758c..de0ba160d5e 100644 --- a/src/kernels/jupyter/serviceRegistry.web.ts +++ b/src/kernels/jupyter/serviceRegistry.web.ts @@ -21,7 +21,6 @@ import { NotebookProvider } from './launcher/notebookProvider'; import { NotebookServerProvider } from './launcher/notebookServerProvider'; import { JupyterServerUriStorage } from './launcher/serverUriStorage'; import { LiveRemoteKernelConnectionUsageTracker } from './liveRemoteKernelConnectionTracker'; -import { RemoteKernelFinder } from './finder/remoteKernelFinder'; import { JupyterServerSelector } from './serverSelector'; import { BackingFileCreator } from './session/backingFileCreator.web'; import { JupyterRequestCreator } from './session/jupyterRequestCreator.web'; @@ -43,16 +42,10 @@ import { IServerConnectionType } from './types'; import { CellOutputMimeTypeTracker } from './jupyterCellOutputMimeTypeTracker'; -import { IConfigurationService } from '../../platform/common/types'; import { UniversalRemoteKernelFinderController } from './finder/universalRemoteKernelFinderController'; export function registerTypes(serviceManager: IServiceManager, _isDevMode: boolean) { serviceManager.addSingleton(IJupyterNotebookProvider, JupyterNotebookProvider); - - serviceManager.addSingleton( - IExtensionSingleActivationService, - RemoteKernelFinder - ); serviceManager.addSingleton(IJupyterExecution, HostJupyterExecution); serviceManager.add(INotebookServerFactory, HostJupyterServerFactory); serviceManager.addSingleton(IJupyterPasswordConnect, JupyterPasswordConnect); @@ -94,16 +87,8 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole IExtensionSyncActivationService, CellOutputMimeTypeTracker ); - const configuration = serviceManager.get(IConfigurationService); - if (configuration.getSettings().kernelPickerType === 'Insiders') { - serviceManager.addSingleton( - IExtensionSingleActivationService, - UniversalRemoteKernelFinderController - ); - } else { - serviceManager.addSingleton( - IExtensionSingleActivationService, - RemoteKernelFinder - ); - } + serviceManager.addSingleton( + IExtensionSingleActivationService, + UniversalRemoteKernelFinderController + ); } diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts index ac13398f155..d731991fb9f 100644 --- a/src/kernels/jupyter/types.ts +++ b/src/kernels/jupyter/types.ts @@ -262,6 +262,8 @@ export interface IJupyterServerUriEntry { export const IJupyterServerUriStorage = Symbol('IJupyterServerUriStorage'); export interface IJupyterServerUriStorage { + isLocalLaunch: boolean; + onDidChangeConnectionType: Event; readonly currentServerId: string | undefined; readonly onDidChangeUri: Event; readonly onDidRemoveUris: Event; diff --git a/src/kernels/raw/finder/localKernelFinder.unit.test.ts b/src/kernels/raw/finder/localKernelFinder.unit.test.ts index 77f9d7e2e04..3c6b13852db 100644 --- a/src/kernels/raw/finder/localKernelFinder.unit.test.ts +++ b/src/kernels/raw/finder/localKernelFinder.unit.test.ts @@ -51,7 +51,7 @@ import { getDisplayPathFromLocalFile } from '../../../platform/common/platform/f import { PythonExtensionChecker } from '../../../platform/api/pythonApi'; import { KernelFinder } from '../../../kernels/kernelFinder'; import { PreferredRemoteKernelIdProvider } from '../../../kernels/jupyter/preferredRemoteKernelIdProvider'; -import { RemoteKernelFinder } from '../../../kernels/jupyter/finder/remoteKernelFinder'; +import { UniversalRemoteKernelFinder } from '../../../kernels/jupyter/finder/universalRemoteKernelFinder'; import { IServerConnectionType } from '../../../kernels/jupyter/types'; import { IPythonExecutionFactory, IPythonExecutionService } from '../../../platform/common/process/types.node'; import { getUserHomeDir } from '../../../platform/common/utils/platform.node'; @@ -66,7 +66,7 @@ import { createEventHandler, TestEventHandler } from '../../../test/common'; [false, true].forEach((isWindows) => { suite(`Local Kernel Finder ${isWindows ? 'Windows' : 'Unix'}`, () => { let localKernelFinder: LocalKernelFinder; - let remoteKernelFinder: RemoteKernelFinder; + let remoteKernelFinder: UniversalRemoteKernelFinder; let kernelFinder: KernelFinder; let interpreterService: IInterpreterService; let platformService: IPlatformService; @@ -110,7 +110,7 @@ import { createEventHandler, TestEventHandler } from '../../../test/common'; const getOSTypeStub = sinon.stub(platform, 'getOSType'); getOSTypeStub.returns(isWindows ? platform.OSType.Windows : platform.OSType.Linux); interpreterService = mock(InterpreterService); - remoteKernelFinder = mock(RemoteKernelFinder); + remoteKernelFinder = mock(UniversalRemoteKernelFinder); onDidChangeInterpreter = new EventEmitter(); onDidChangeInterpreters = new EventEmitter(); disposables.push(onDidChangeInterpreter); From 489327cc9f3ccbc3db736f3e9e1446117f0f95c2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 2 Nov 2022 18:23:50 -0700 Subject: [PATCH 2/4] :lipstick: --- .../jupyter/finder/universalRemoteKernelFinder.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts b/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts index 9dd831b0933..abf711433a9 100644 --- a/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts +++ b/src/kernels/jupyter/finder/universalRemoteKernelFinder.ts @@ -66,9 +66,9 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri private wasPythonInstalledWhenFetchingKernels = false; constructor( - readonly id: string /**`${this.kind}-${serverUri.serverId}` */, - readonly displayName: string /** localize.DataScience.universalRemoteKernelFinderDisplayName().format( serverUri.displayName || serverUri.uri ) */, - readonly cacheKey: string /** `${RemoteKernelSpecsCacheKey}-${this.serverUri.serverId}` */, + readonly id: string, + readonly displayName: string, + readonly cacheKey: string, private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, private interpreterService: IInterpreterService, private extensionChecker: IPythonExtensionChecker, @@ -236,7 +236,7 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri traceVerbose('UniversalRemoteKernelFinder: get from cache'); let results: RemoteKernelConnectionMetadata[] = this.cache; - const key = this.getCacheKey(); + const key = this.cacheKey; // If not in memory, check memento if (!results || results.length === 0) { @@ -381,17 +381,13 @@ export class UniversalRemoteKernelFinder implements IRemoteKernelFinder, IContri } } - private getCacheKey() { - return this.cacheKey; - } - private async writeToCache(values: RemoteKernelConnectionMetadata[]) { try { traceVerbose( `UniversalRemoteKernelFinder: Writing ${values.length} remote kernel connection metadata to cache` ); - const key = this.getCacheKey(); + const key = this.cacheKey; this.cache = values; const serialized = values.map(serializeKernelConnection); await Promise.all([ From b00aac5d2d220fc4facea363746c009417c00bd6 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 2 Nov 2022 18:30:29 -0700 Subject: [PATCH 3/4] life cycle management. --- .../universalRemoteKernelFinderController.ts | 97 +++++++++++-------- 1 file changed, 56 insertions(+), 41 deletions(-) diff --git a/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts b/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts index 58c7b703786..beb3bf2f8ae 100644 --- a/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts +++ b/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts @@ -34,6 +34,7 @@ import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder'; /** Strategy design */ interface IRemoteKernelFinderRegistrationStrategy { activate(): Promise; + dispose(): void; } class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { @@ -112,6 +113,10 @@ class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { this.serverFinderMapping.delete(uri.serverId); }); } + + dispose() { + this.serverFinderMapping.forEach((finder) => finder.dispose()); + } } class SingleServerStrategy implements IRemoteKernelFinderRegistrationStrategy { @@ -190,6 +195,10 @@ class SingleServerStrategy implements IRemoteKernelFinderRegistrationStrategy { finder.activate().then(noop, noop); } + + dispose() { + this._activeServerFinder?.finder.dispose(); + } } // This class creates RemoteKernelFinders for all registered Jupyter Server URIs @@ -199,52 +208,58 @@ export class UniversalRemoteKernelFinderController implements IExtensionSingleAc constructor( @inject(IConfigurationService) readonly configurationService: IConfigurationService, - @inject(IJupyterSessionManagerFactory) jupyterSessionManagerFactory: IJupyterSessionManagerFactory, - @inject(IInterpreterService) interpreterService: IInterpreterService, - @inject(IPythonExtensionChecker) extensionChecker: IPythonExtensionChecker, - @inject(INotebookProvider) notebookProvider: INotebookProvider, - @inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage, - @inject(IMemento) @named(GLOBAL_MEMENTO) globalState: Memento, - @inject(IApplicationEnvironment) env: IApplicationEnvironment, + @inject(IJupyterSessionManagerFactory) + private readonly jupyterSessionManagerFactory: IJupyterSessionManagerFactory, + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, + @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker, + @inject(INotebookProvider) private readonly notebookProvider: INotebookProvider, + @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage, + @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento, + @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, @inject(IJupyterRemoteCachedKernelValidator) - cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, - @inject(IKernelFinder) kernelFinder: KernelFinder, - @inject(IDisposableRegistry) disposables: IDisposableRegistry, - @inject(IKernelProvider) kernelProvider: IKernelProvider, - @inject(IExtensions) extensions: IExtensions, - @inject(IsWebExtension) isWebExtension: boolean + private readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, + @inject(IKernelFinder) private readonly kernelFinder: KernelFinder, + @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, + @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, + @inject(IExtensions) private readonly extensions: IExtensions, + @inject(IsWebExtension) private readonly isWebExtension: boolean ) { + this._strategy = this.getStrategy(); + this.disposables.push(this._strategy); + } + + private getStrategy(): IRemoteKernelFinderRegistrationStrategy { if (this.configurationService.getSettings().kernelPickerType === 'Insiders') { - this._strategy = new MultiServerStrategy( - jupyterSessionManagerFactory, - interpreterService, - extensionChecker, - notebookProvider, - serverUriStorage, - globalState, - env, - cachedRemoteKernelValidator, - kernelFinder, - disposables, - kernelProvider, - extensions, - isWebExtension + return new MultiServerStrategy( + this.jupyterSessionManagerFactory, + this.interpreterService, + this.extensionChecker, + this.notebookProvider, + this.serverUriStorage, + this.globalState, + this.env, + this.cachedRemoteKernelValidator, + this.kernelFinder, + this.disposables, + this.kernelProvider, + this.extensions, + this.isWebExtension ); } else { - this._strategy = new SingleServerStrategy( - jupyterSessionManagerFactory, - interpreterService, - extensionChecker, - notebookProvider, - serverUriStorage, - globalState, - env, - cachedRemoteKernelValidator, - kernelFinder, - disposables, - kernelProvider, - extensions, - isWebExtension + return new SingleServerStrategy( + this.jupyterSessionManagerFactory, + this.interpreterService, + this.extensionChecker, + this.notebookProvider, + this.serverUriStorage, + this.globalState, + this.env, + this.cachedRemoteKernelValidator, + this.kernelFinder, + this.disposables, + this.kernelProvider, + this.extensions, + this.isWebExtension ); } } From c941fd3f6e7823ca3178c090344ec348f8882367 Mon Sep 17 00:00:00 2001 From: rebornix Date: Thu, 3 Nov 2022 13:59:30 -0700 Subject: [PATCH 4/4] Comments. --- .../jupyter/finder/universalRemoteKernelFinderController.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts b/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts index beb3bf2f8ae..3f0713afed5 100644 --- a/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts +++ b/src/kernels/jupyter/finder/universalRemoteKernelFinderController.ts @@ -73,7 +73,7 @@ class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { createRemoteKernelFinder(serverUri: IJupyterServerUriEntry) { if (!serverUri.isValidated) { - // TODO@rebornix, what if it's now validated? + // when server uri is validated, an `onDidAddUri` event will be fired. return; }