diff --git a/src/kernels/common/baseJupyterSession.ts b/src/kernels/common/baseJupyterSession.ts index 8d64f870dfe..e270a1a470c 100644 --- a/src/kernels/common/baseJupyterSession.ts +++ b/src/kernels/common/baseJupyterSession.ts @@ -96,6 +96,7 @@ export abstract class BaseJupyterSession implements IBaseKernelConnectionSession private _wrappedKernel?: KernelConnectionWrapper; private _isDisposed?: boolean; private readonly _disposed = new EventEmitter(); + private readonly didShutdown = new EventEmitter(); protected readonly disposables: IDisposable[] = []; public get disposed() { return this._isDisposed === true; @@ -103,6 +104,9 @@ export abstract class BaseJupyterSession implements IBaseKernelConnectionSession public get onDidDispose() { return this._disposed.event; } + public get onDidShutdown() { + return this.didShutdown.event; + } protected get session(): ISessionWithSocket | undefined { return this._session; } @@ -525,6 +529,8 @@ export abstract class BaseJupyterSession implements IBaseKernelConnectionSession this.restartSessionPromise = undefined; this.onStatusChangedEvent.fire('dead'); this._disposed.fire(); + this.didShutdown.fire(); + this.didShutdown.dispose(); this._disposed.dispose(); this.onStatusChangedEvent.dispose(); this.previousAnyMessageHandler?.dispose(); diff --git a/src/kernels/common/kernelConnectionSessionCreator.ts b/src/kernels/common/kernelConnectionSessionCreator.ts new file mode 100644 index 00000000000..d466167cd08 --- /dev/null +++ b/src/kernels/common/kernelConnectionSessionCreator.ts @@ -0,0 +1,160 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { inject, injectable, optional } from 'inversify'; +import { + IJupyterConnection, + IKernelConnectionSession, + IKernelConnectionSessionCreator, + isLocalConnection, + isRemoteConnection, + KernelConnectionSessionCreationOptions +} from '../types'; +import { Cancellation } from '../../platform/common/cancellation'; +import { IRawKernelConnectionSessionCreator } from '../raw/types'; +import { IJupyterServerProvider, IJupyterSessionManagerFactory } from '../jupyter/types'; +import { JupyterKernelConnectionSessionCreator } from '../jupyter/session/jupyterKernelConnectionSessionCreator'; +import { JupyterConnection } from '../jupyter/connection/jupyterConnection'; +import { Telemetry, sendTelemetryEvent } from '../../telemetry'; +import { JupyterSelfCertsError } from '../../platform/errors/jupyterSelfCertsError'; +import { JupyterSelfCertsExpiredError } from '../../platform/errors/jupyterSelfCertsExpiredError'; +import { RemoteJupyterServerConnectionError } from '../../platform/errors/remoteJupyterServerConnectionError'; +import { disposeAllDisposables } from '../../platform/common/helpers'; +import { IAsyncDisposableRegistry, IDisposable } from '../../platform/common/types'; +import { KernelProgressReporter } from '../../platform/progress/kernelProgressReporter'; +import { DataScience } from '../../platform/common/utils/localize'; +import { Disposable } from 'vscode'; +import { noop } from '../../platform/common/utils/misc'; +import { LocalJupyterServerConnectionError } from '../../platform/errors/localJupyterServerConnectionError'; +import { BaseError } from '../../platform/errors/types'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ +const LocalHosts = ['localhost', '127.0.0.1', '::1']; + +/** + * Generic class for connecting to a server. Probably could be renamed as it doesn't provide notebooks, but rather connections. + */ +@injectable() +export class KernelConnectionSessionCreator implements IKernelConnectionSessionCreator { + constructor( + @inject(IRawKernelConnectionSessionCreator) + @optional() + private readonly rawKernelSessionCreator: IRawKernelConnectionSessionCreator | undefined, + @inject(IJupyterServerProvider) + private readonly jupyterNotebookProvider: IJupyterServerProvider, + @inject(IJupyterSessionManagerFactory) private readonly sessionManagerFactory: IJupyterSessionManagerFactory, + @inject(JupyterKernelConnectionSessionCreator) + private readonly jupyterSessionCreator: JupyterKernelConnectionSessionCreator, + @inject(JupyterConnection) private readonly jupyterConnection: JupyterConnection, + @inject(IAsyncDisposableRegistry) private readonly asyncDisposables: IAsyncDisposableRegistry + ) {} + + public async create(options: KernelConnectionSessionCreationOptions): Promise { + const kernelConnection = options.kernelConnection; + const isLocal = isLocalConnection(kernelConnection); + + if (this.rawKernelSessionCreator?.isSupported && isLocal) { + return this.createRawKernelSession(this.rawKernelSessionCreator, options); + } + + const disposables: IDisposable[] = []; + let progressReporter: IDisposable | undefined; + const createProgressReporter = () => { + if (options.ui.disableUI || progressReporter) { + return; + } + // Status depends upon if we're about to connect to existing server or not. + progressReporter = KernelProgressReporter.createProgressReporter( + options.resource, + isRemoteConnection(options.kernelConnection) + ? DataScience.connectingToJupyter + : DataScience.startingJupyter + ); + disposables.push(progressReporter); + }; + if (options.ui.disableUI) { + options.ui.onDidChangeDisableUI(createProgressReporter, this, disposables); + } + createProgressReporter(); + + return this.createJupyterKernelSession(options).finally(() => disposeAllDisposables(disposables)); + } + private createRawKernelSession( + factory: IRawKernelConnectionSessionCreator, + options: KernelConnectionSessionCreationOptions + ): Promise { + return factory.create(options.resource, options.kernelConnection, options.ui, options.token); + } + private async createJupyterKernelSession( + options: KernelConnectionSessionCreationOptions + ): Promise { + let connection: undefined | IJupyterConnection; + + // Check to see if we support ipykernel or not + const disposablesWhenThereAreFailures: IDisposable[] = []; + try { + connection = isRemoteConnection(options.kernelConnection) + ? await this.jupyterConnection.createConnectionInfo({ + serverId: options.kernelConnection.serverId + }) + : await this.jupyterNotebookProvider.getOrCreateServer({ + resource: options.resource, + token: options.token, + ui: options.ui + }); + + if (!connection.localLaunch && LocalHosts.includes(connection.hostName.toLowerCase())) { + sendTelemetryEvent(Telemetry.ConnectRemoteJupyterViaLocalHost); + } + + Cancellation.throwIfCanceled(options.token); + + const sessionManager = await this.sessionManagerFactory.create(connection); + this.asyncDisposables.push(sessionManager); + disposablesWhenThereAreFailures.push(new Disposable(() => sessionManager.dispose().catch(noop))); + + Cancellation.throwIfCanceled(options.token); + // Disposing session manager will dispose all sessions that were started by that session manager. + // Hence Session managers should be disposed only if the corresponding session is shutdown. + const session = await this.jupyterSessionCreator.create({ + creator: options.creator, + kernelConnection: options.kernelConnection, + resource: options.resource, + sessionManager, + token: options.token, + ui: options.ui + }); + session.onDidShutdown(() => sessionManager.dispose()); + return session; + } catch (ex) { + disposeAllDisposables(disposablesWhenThereAreFailures); + + if (isRemoteConnection(options.kernelConnection)) { + sendTelemetryEvent(Telemetry.ConnectRemoteFailedJupyter, undefined, undefined, ex); + // Check for the self signed certs error specifically + if (!connection) { + throw ex; + } else if (JupyterSelfCertsError.isSelfCertsError(ex)) { + sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter); + throw new JupyterSelfCertsError(connection.baseUrl); + } else if (JupyterSelfCertsExpiredError.isSelfCertsExpiredError(ex)) { + sendTelemetryEvent(Telemetry.ConnectRemoteExpiredCertFailedJupyter); + throw new JupyterSelfCertsExpiredError(connection.baseUrl); + } else { + throw new RemoteJupyterServerConnectionError( + connection.baseUrl, + options.kernelConnection.serverId, + ex + ); + } + } else { + sendTelemetryEvent(Telemetry.ConnectFailedJupyter, undefined, undefined, ex); + if (ex instanceof BaseError) { + throw ex; + } else { + throw new LocalJupyterServerConnectionError(ex); + } + } + } + } +} diff --git a/src/kernels/jupyter/launcher/kernelConnectionSessionCreator.unit.test.ts b/src/kernels/common/kernelConnectionSessionCreator.unit.test.ts similarity index 58% rename from src/kernels/jupyter/launcher/kernelConnectionSessionCreator.unit.test.ts rename to src/kernels/common/kernelConnectionSessionCreator.unit.test.ts index 74ffd94413d..a76884bf78e 100644 --- a/src/kernels/jupyter/launcher/kernelConnectionSessionCreator.unit.test.ts +++ b/src/kernels/common/kernelConnectionSessionCreator.unit.test.ts @@ -1,16 +1,21 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +import { IDisposable } from '@fluentui/react'; import { expect } from 'chai'; import { anything, instance, mock, when } from 'ts-mockito'; import * as vscode from 'vscode'; -import { PythonExtensionChecker } from '../../../platform/api/pythonApi'; -import { IJupyterKernelConnectionSession, KernelConnectionMetadata } from '../../types'; -import { DisplayOptions } from '../../displayOptions'; -import { IJupyterNotebookProvider } from '../types'; -import { IRawKernelConnectionSessionCreator } from '../../raw/types'; -import { IDisposable } from '../../../platform/common/types'; -import { disposeAllDisposables } from '../../../platform/common/helpers'; +import { EventEmitter } from 'vscode'; +import { PythonExtensionChecker } from '../../platform/api/pythonApi'; +import { AsyncDisposableRegistry } from '../../platform/common/asyncDisposableRegistry'; +import { disposeAllDisposables } from '../../platform/common/helpers'; +import { IAsyncDisposableRegistry } from '../../platform/common/types'; +import { DisplayOptions } from '../displayOptions'; +import { JupyterConnection } from '../jupyter/connection/jupyterConnection'; +import { JupyterKernelConnectionSessionCreator } from '../jupyter/session/jupyterKernelConnectionSessionCreator'; +import { IJupyterServerProvider, IJupyterSessionManagerFactory } from '../jupyter/types'; +import { IRawKernelConnectionSessionCreator } from '../raw/types'; +import { IJupyterKernelConnectionSession, KernelConnectionMetadata } from '../types'; import { KernelConnectionSessionCreator } from './kernelConnectionSessionCreator'; function Uri(filename: string): vscode.Uri { @@ -20,12 +25,14 @@ function Uri(filename: string): vscode.Uri { /* eslint-disable */ suite('NotebookProvider', () => { let kernelConnectionSessionCreator: KernelConnectionSessionCreator; - let jupyterNotebookProvider: IJupyterNotebookProvider; + let jupyterNotebookProvider: IJupyterServerProvider; let rawKernelSessionCreator: IRawKernelConnectionSessionCreator; let cancelToken: vscode.CancellationTokenSource; const disposables: IDisposable[] = []; + let asyncDisposables: IAsyncDisposableRegistry; + let onDidShutdown: EventEmitter; setup(() => { - jupyterNotebookProvider = mock(); + jupyterNotebookProvider = mock(); rawKernelSessionCreator = mock(); cancelToken = new vscode.CancellationTokenSource(); disposables.push(cancelToken); @@ -34,21 +41,37 @@ suite('NotebookProvider', () => { when(extensionChecker.isPythonExtensionInstalled).thenReturn(true); const onDidChangeEvent = new vscode.EventEmitter(); disposables.push(onDidChangeEvent); - + onDidShutdown = new vscode.EventEmitter(); + disposables.push(onDidShutdown); + const sessionManagerFactory = mock(); + const jupyterSessionCreator = mock(); + const jupyterConnection = mock(); + when(jupyterConnection.createConnectionInfo(anything())).thenResolve({ + localLaunch: true, + baseUrl: 'http://localhost:8888' + } as any); + const mockSession = mock(); + when(mockSession.onDidShutdown).thenReturn(onDidShutdown.event); + instance(mockSession as any).then = undefined; + when(jupyterSessionCreator.create(anything())).thenResolve(instance(mockSession)); + asyncDisposables = new AsyncDisposableRegistry(); kernelConnectionSessionCreator = new KernelConnectionSessionCreator( instance(rawKernelSessionCreator), - instance(jupyterNotebookProvider) + instance(jupyterNotebookProvider), + instance(sessionManagerFactory), + instance(jupyterSessionCreator), + instance(jupyterConnection), + asyncDisposables ); }); - teardown(() => disposeAllDisposables(disposables)); + teardown(async () => { + disposeAllDisposables(disposables); + await asyncDisposables.dispose(); + }); test('NotebookProvider getOrCreateNotebook jupyter provider does not have notebook already', async () => { - const mockSession = mock(); - instance(mockSession as any).then = undefined; - when(jupyterNotebookProvider.createNotebook(anything())).thenResolve(instance(mockSession)); - when(jupyterNotebookProvider.connect(anything())).thenResolve({} as any); + when(jupyterNotebookProvider.getOrCreateServer(anything())).thenResolve({} as any); const doc = mock(); when(doc.uri).thenReturn(Uri('C:\\\\foo.py')); - const session = await kernelConnectionSessionCreator.create({ resource: Uri('C:\\\\foo.py'), kernelConnection: instance(mock()), @@ -60,10 +83,7 @@ suite('NotebookProvider', () => { }); test('NotebookProvider getOrCreateNotebook second request should return the notebook already cached', async () => { - const mockSession = mock(); - instance(mockSession as any).then = undefined; - when(jupyterNotebookProvider.createNotebook(anything())).thenResolve(instance(mockSession)); - when(jupyterNotebookProvider.connect(anything())).thenResolve({} as any); + when(jupyterNotebookProvider.getOrCreateServer(anything())).thenResolve({} as any); const doc = mock(); when(doc.uri).thenReturn(Uri('C:\\\\foo.py')); diff --git a/src/kernels/jupyter/finder/remoteKernelFinder.ts b/src/kernels/jupyter/finder/remoteKernelFinder.ts index d55d671239b..b3045841524 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinder.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinder.ts @@ -6,7 +6,6 @@ import { getKernelId } from '../../helpers'; import { BaseKernelConnectionMetadata, IJupyterKernelSpec, - IJupyterServerConnector, IKernelProvider, IJupyterConnection, isRemoteConnection, @@ -27,7 +26,6 @@ import { traceError, traceWarning, traceInfoIfCI, traceVerbose } from '../../../ import { IPythonExtensionChecker } from '../../../platform/api/types'; import { computeServerId } from '../jupyterUtils'; import { createPromiseFromCancellation } from '../../../platform/common/cancellation'; -import { DisplayOptions } from '../../displayOptions'; import { isArray } from '../../../platform/common/utils/sysTypes'; import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc'; import { IApplicationEnvironment } from '../../../platform/common/application/types'; @@ -37,6 +35,10 @@ import { ContributedKernelFinderKind } from '../../internalTypes'; import { disposeAllDisposables } from '../../../platform/common/helpers'; import { PromiseMonitor } from '../../../platform/common/utils/promises'; import { getDisplayPath } from '../../../platform/common/platform/fs-paths'; +import { JupyterConnection } from '../connection/jupyterConnection'; +import { KernelProgressReporter } from '../../../platform/progress/kernelProgressReporter'; +import { DataScience } from '../../../platform/common/utils/localize'; +import { isUnitTestExecution } from '../../../platform/common/constants'; // 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. @@ -90,14 +92,14 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable { readonly cacheKey: string, private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, private extensionChecker: IPythonExtensionChecker, - private readonly jupyterServerConnector: IJupyterServerConnector, private readonly globalState: Memento, private readonly env: IApplicationEnvironment, private readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, kernelFinder: KernelFinder, private readonly kernelProvider: IKernelProvider, private readonly extensions: IExtensions, - readonly serverUri: IJupyterServerUriEntry + readonly serverUri: IJupyterServerUriEntry, + private readonly jupyterConnection: JupyterConnection ) { // 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 @@ -200,7 +202,7 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable { } else { try { const kernelsWithoutCachePromise = (async () => { - const connInfo = await this.getRemoteConnectionInfo(undefined, displayProgress); + const connInfo = await this.getRemoteConnectionInfo(displayProgress); return connInfo ? this.listKernelsFromConnection(connInfo) : Promise.resolve([]); })(); @@ -227,7 +229,7 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable { try { const kernelsWithoutCachePromise = (async () => { - const connInfo = await this.getRemoteConnectionInfo(updateCacheCancellationToken.token, false); + const connInfo = await this.getRemoteConnectionInfo(false); return connInfo ? this.listKernelsFromConnection(connInfo) : Promise.resolve([]); })(); @@ -260,18 +262,16 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable { return this.cache; } - private async getRemoteConnectionInfo( - cancelToken?: CancellationToken, - displayProgress: boolean = true - ): Promise { - const ui = new DisplayOptions(!displayProgress); - return this.jupyterServerConnector.connect({ - resource: undefined, - ui, - localJupyter: false, - token: cancelToken, - serverId: this.serverUri.serverId - }); + private async getRemoteConnectionInfo(displayProgress: boolean = true): Promise { + const disposables: IDisposable[] = []; + if (displayProgress) { + disposables.push(KernelProgressReporter.createProgressReporter(undefined, DataScience.connectingToJupyter)); + } + return this.jupyterConnection + .createConnectionInfo({ + serverId: this.serverUri.serverId + }) + .finally(() => disposeAllDisposables(disposables)); } private async getFromCache(cancelToken?: CancellationToken): Promise { @@ -435,19 +435,30 @@ export class RemoteKernelFinder implements IRemoteKernelFinder, IDisposable { clearTimeout(this.cacheLoggingTimeout); } // Reduce the logging, as this can get written a lot, - this.cacheLoggingTimeout = setTimeout(() => { - traceVerbose( - `Updating cache with Remote kernels ${values - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Added = ${added - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Updated = ${updated - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Removed = ${removed - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}` - ); - }, 15_000); + this.cacheLoggingTimeout = setTimeout( + () => { + traceVerbose( + `Updating cache with Remote kernels ${values + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Added = ${added + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Updated = ${updated + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Removed = ${removed + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}` + ); + }, + isUnitTestExecution() ? 0 : 15_000 + ); this.disposables.push(new Disposable(() => clearTimeout(this.cacheLoggingTimeout))); } } catch (ex) { diff --git a/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts b/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts index df2cc00219a..123838724f9 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts @@ -33,6 +33,8 @@ import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder'; import { IExtensions } from '../../../platform/common/types'; import { createEventHandler, TestEventHandler } from '../../../test/common'; import { RemoteKernelFinder } from './remoteKernelFinder'; +import { JupyterConnection } from '../connection/jupyterConnection'; +import { disposeAllDisposables } from '../../../platform/common/helpers'; suite(`Remote Kernel Finder`, () => { let disposables: Disposable[] = []; @@ -44,6 +46,7 @@ suite(`Remote Kernel Finder`, () => { const dummyEvent = new EventEmitter(); let cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator; let kernelsChanged: TestEventHandler; + let jupyterConnection: JupyterConnection; const connInfo: IJupyterConnection = { url: 'http://foobar', type: 'jupyter', @@ -151,27 +154,26 @@ suite(`Remote Kernel Finder`, () => { kernelFinder = new KernelFinder(disposables); kernelsChanged = createEventHandler(kernelFinder, 'onDidChangeKernels'); disposables.push(kernelsChanged); - + jupyterConnection = mock(); + when(jupyterConnection.createConnectionInfo(anything())).thenResolve(connInfo); remoteKernelFinder = new RemoteKernelFinder( 'currentremote', 'Local Kernels', RemoteKernelSpecsCacheKey, instance(jupyterSessionManagerFactory), instance(extensionChecker), - instance(serverConnector), instance(memento), instance(env), instance(cachedRemoteKernelValidator), kernelFinder, instance(kernelProvider), instance(extensions), - serverEntry + serverEntry, + instance(jupyterConnection) ); remoteKernelFinder.activate().then(noop, noop); }); - teardown(() => { - disposables.forEach((d) => d.dispose()); - }); + teardown(() => disposeAllDisposables(disposables)); test('Kernels found', async () => { when(jupyterSessionManager.getRunningKernels()).thenResolve([]); when(jupyterSessionManager.getRunningSessions()).thenResolve([]); diff --git a/src/kernels/jupyter/finder/remoteKernelFinderController.ts b/src/kernels/jupyter/finder/remoteKernelFinderController.ts index 33e995604ff..90aaee69717 100644 --- a/src/kernels/jupyter/finder/remoteKernelFinderController.ts +++ b/src/kernels/jupyter/finder/remoteKernelFinderController.ts @@ -3,7 +3,7 @@ import { injectable, inject, named } from 'inversify'; import { Disposable, Memento } from 'vscode'; -import { IKernelFinder, IKernelProvider, IJupyterServerConnector } from '../../types'; +import { IKernelFinder, IKernelProvider } from '../../types'; import { GLOBAL_MEMENTO, IDisposableRegistry, IExtensions, IMemento } from '../../../platform/common/types'; import { IJupyterSessionManagerFactory, @@ -21,6 +21,7 @@ import { ContributedKernelFinderKind } from '../../internalTypes'; import * as localize from '../../../platform/common/utils/localize'; import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder'; import { Settings } from '../../../platform/common/constants'; +import { JupyterConnection } from '../connection/jupyterConnection'; /** Strategy design */ interface IRemoteKernelFinderRegistrationStrategy { @@ -34,7 +35,6 @@ class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { constructor( private jupyterSessionManagerFactory: IJupyterSessionManagerFactory, private extensionChecker: IPythonExtensionChecker, - private readonly jupyterServerConnector: IJupyterServerConnector, private readonly serverUriStorage: IJupyterServerUriStorage, private readonly globalState: Memento, private readonly env: IApplicationEnvironment, @@ -42,7 +42,8 @@ class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { private readonly kernelFinder: KernelFinder, private readonly disposables: IDisposableRegistry, private readonly kernelProvider: IKernelProvider, - private readonly extensions: IExtensions + private readonly extensions: IExtensions, + private readonly jupyterConnection: JupyterConnection ) {} async activate(): Promise { @@ -75,14 +76,14 @@ class MultiServerStrategy implements IRemoteKernelFinderRegistrationStrategy { `${RemoteKernelSpecsCacheKey}-${serverUri.serverId}`, this.jupyterSessionManagerFactory, this.extensionChecker, - this.jupyterServerConnector, this.globalState, this.env, this.cachedRemoteKernelValidator, this.kernelFinder, this.kernelProvider, this.extensions, - serverUri + serverUri, + this.jupyterConnection ); this.disposables.push(finder); @@ -116,7 +117,6 @@ export class RemoteKernelFinderController implements IExtensionSyncActivationSer @inject(IJupyterSessionManagerFactory) private readonly jupyterSessionManagerFactory: IJupyterSessionManagerFactory, @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker, - @inject(IJupyterServerConnector) private readonly jupyterServerConnector: IJupyterServerConnector, @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage, @inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento, @inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment, @@ -124,12 +124,12 @@ export class RemoteKernelFinderController implements IExtensionSyncActivationSer private readonly cachedRemoteKernelValidator: IJupyterRemoteCachedKernelValidator, @inject(IKernelFinder) private readonly kernelFinder: KernelFinder, @inject(IKernelProvider) private readonly kernelProvider: IKernelProvider, - @inject(IExtensions) private readonly extensions: IExtensions + @inject(IExtensions) private readonly extensions: IExtensions, + @inject(JupyterConnection) jupyterConnection: JupyterConnection ) { this._strategy = new MultiServerStrategy( this.jupyterSessionManagerFactory, this.extensionChecker, - this.jupyterServerConnector, this.serverUriStorage, this.globalState, this.env, @@ -137,7 +137,8 @@ export class RemoteKernelFinderController implements IExtensionSyncActivationSer this.kernelFinder, this._localDisposables, this.kernelProvider, - this.extensions + this.extensions, + jupyterConnection ); } diff --git a/src/kernels/jupyter/launcher/hostJupyterExecution.ts b/src/kernels/jupyter/launcher/hostJupyterExecution.ts new file mode 100644 index 00000000000..b6167017beb --- /dev/null +++ b/src/kernels/jupyter/launcher/hostJupyterExecution.ts @@ -0,0 +1,217 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import uuid from 'uuid/v4'; +import { CancellationToken, Uri } from 'vscode'; +import { inject, injectable, optional } from 'inversify'; +import { IWorkspaceService } from '../../../platform/common/application/types'; +import { traceError, traceInfo, traceVerbose } from '../../../platform/logging'; +import { + IDisposableRegistry, + IAsyncDisposableRegistry, + IConfigurationService, + Resource +} from '../../../platform/common/types'; +import { IInterpreterService } from '../../../platform/interpreter/contracts'; +import { IJupyterExecution, INotebookStarter } from '../types'; +import * as urlPath from '../../../platform/vscode-path/resources'; +import { IJupyterSubCommandExecutionService } from '../types.node'; +import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; +import { DataScience } from '../../../platform/common/utils/localize'; +import { Cancellation } from '../../../platform/common/cancellation'; +import { IJupyterConnection } from '../../types'; +import { JupyterWaitForIdleError } from '../../errors/jupyterWaitForIdleError'; +import { expandWorkingDir } from '../jupyterUtils'; +import { noop } from '../../../platform/common/utils/misc'; + +/** + * Jupyter server implementation that uses the JupyterExecutionBase class to launch Jupyter. + */ +@injectable() +export class HostJupyterExecution implements IJupyterExecution { + private usablePythonInterpreter: PythonEnvironment | undefined; + private cache?: Promise; + private disposed: boolean = false; + private _disposed = false; + constructor( + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, + @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry, + @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, + @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, + @inject(IConfigurationService) private readonly configuration: IConfigurationService, + @inject(INotebookStarter) @optional() private readonly notebookStarter: INotebookStarter | undefined, + @inject(IJupyterSubCommandExecutionService) + @optional() + private readonly jupyterInterpreterService: IJupyterSubCommandExecutionService | undefined + ) { + this.disposableRegistry.push(this.interpreterService.onDidChangeInterpreter(() => this.onSettingsChanged())); + this.disposableRegistry.push(this); + + if (workspace) { + workspace.onDidChangeConfiguration( + (e) => { + if (e.affectsConfiguration('python.dataScience', undefined)) { + // When config changes happen, recreate our commands. + this.onSettingsChanged(); + } + }, + this, + this.disposableRegistry + ); + } + asyncRegistry.push(this); + } + + public async dispose(): Promise { + traceInfo(`Disposing HostJupyterExecution`); + if (!this._disposed) { + this._disposed = true; + traceVerbose(`Disposing super HostJupyterExecution`); + this.disposed = true; + + // Cleanup on dispose. We are going away permanently + traceVerbose(`Cleaning up server cache`); + await this.cache?.then((s) => s.dispose()).catch(noop); + } + traceVerbose(`Finished disposing HostJupyterExecution`); + } + + private async hostConnectToNotebookServer( + resource: Resource, + cancelToken: CancellationToken + ): Promise { + if (!this._disposed) { + return this.connectToNotebookServerImpl(resource, cancelToken); + } + throw new Error('Notebook server is disposed'); + } + + public async connectToNotebookServer( + resource: Resource, + cancelToken: CancellationToken + ): Promise { + if (this._disposed) { + throw new Error('Notebook server is disposed'); + } + if (!this.cache) { + const promise = (this.cache = this.hostConnectToNotebookServer(resource, cancelToken)); + promise.catch((ex) => { + traceError(`Failed to start the Jupyter Server`, ex); + if (this.cache === promise) { + this.cache = undefined; + } + }); + } + + return this.cache; + } + public async getServer(): Promise { + return !this._disposed && this.cache ? this.cache : undefined; + } + + public async refreshCommands(): Promise { + await this.jupyterInterpreterService?.refreshCommands(); + } + + public async isNotebookSupported(cancelToken?: CancellationToken): Promise { + // See if we can find the command notebook + return this.jupyterInterpreterService ? this.jupyterInterpreterService.isNotebookSupported(cancelToken) : false; + } + + public async getNotebookError(): Promise { + return this.jupyterInterpreterService + ? this.jupyterInterpreterService.getReasonForJupyterNotebookNotBeingSupported() + : DataScience.webNotSupported; + } + + public async getUsableJupyterPython(cancelToken?: CancellationToken): Promise { + // Only try to compute this once. + if (!this.usablePythonInterpreter && !this.disposed && this.jupyterInterpreterService) { + this.usablePythonInterpreter = await Cancellation.race( + () => this.jupyterInterpreterService!.getSelectedInterpreter(cancelToken), + cancelToken + ); + } + return this.usablePythonInterpreter; + } + + /* eslint-disable complexity, */ + public connectToNotebookServerImpl( + resource: Resource, + cancelToken: CancellationToken + ): Promise { + // Return nothing if we cancel + // eslint-disable-next-line + return Cancellation.race(async () => { + let connection: IJupyterConnection | undefined; + + // Try to connect to our jupyter process. Check our setting for the number of tries + let tryCount = 1; + const maxTries = Math.max(1, this.configuration.getSettings(undefined).jupyterLaunchRetries); + let lastTryError: Error; + while (tryCount <= maxTries && !this.disposed) { + try { + // Start or connect to the process + connection = await this.start(resource, cancelToken); + + traceVerbose(`Connection complete server`); + return connection; + } catch (err) { + lastTryError = err; + if (err instanceof JupyterWaitForIdleError && tryCount < maxTries) { + // Special case. This sometimes happens where jupyter doesn't ever connect. Cleanup after + // ourselves and propagate the failure outwards. + traceInfo('Retry because of wait for idle problem.'); + + // Close existing connection. + connection?.dispose(); + tryCount += 1; + } else if (connection) { + // If this is occurring during shutdown, don't worry about it. + if (this.disposed) { + throw err; + } + throw err; + } else { + throw err; + } + } + throw lastTryError; + } + throw new Error('Max number of attempts reached'); + }, cancelToken); + } + + private async start(resource: Resource, cancelToken: CancellationToken): Promise { + // If our uri is undefined or if it's set to local launch we need to launch a server locally + // If that works, then attempt to start the server + traceInfo(`Launching server`); + const settings = this.configuration.getSettings(resource); + const useDefaultConfig = settings.useDefaultConfigForJupyter; + const workingDir = await this.workspace.computeWorkingDirectory(resource); + // Expand the working directory. Create a dummy launching file in the root path (so we expand correctly) + const workingDirectory = expandWorkingDir( + workingDir, + this.workspace.rootFolder ? urlPath.joinPath(this.workspace.rootFolder, `${uuid()}.txt`) : undefined, + this.workspace, + settings + ); + + if (!this.notebookStarter) { + // In desktop mode this must be defined, in web this code path never gets executed. + throw new Error('Notebook Starter cannot be undefined'); + } + return this.notebookStarter.start( + resource, + useDefaultConfig, + this.configuration.getSettings(undefined).jupyterCommandLineArguments, + Uri.file(workingDirectory), + cancelToken + ); + } + + private onSettingsChanged() { + // Clear our usableJupyterInterpreter so that we recompute our values + this.usablePythonInterpreter = undefined; + } +} diff --git a/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts b/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts deleted file mode 100644 index a3500dfe619..00000000000 --- a/src/kernels/jupyter/launcher/jupyterNotebookProvider.ts +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { inject, injectable } from 'inversify'; -import { - ConnectNotebookProviderOptions, - GetServerOptions, - IJupyterConnection, - IKernelConnectionSession, - isLocalConnection, - NotebookCreationOptions -} from '../../types'; -import { Cancellation } from '../../../platform/common/cancellation'; -import { IJupyterNotebookProvider, IJupyterServerProvider } from '../types'; - -// When the NotebookProvider looks to create a notebook it uses this class to create a Jupyter notebook -@injectable() -export class JupyterNotebookProvider implements IJupyterNotebookProvider { - constructor(@inject(IJupyterServerProvider) private readonly serverProvider: IJupyterServerProvider) {} - - public async connect(options: ConnectNotebookProviderOptions): Promise { - const { connection } = await this.serverProvider.getOrCreateServer(options); - return connection; - } - - public async createNotebook(options: NotebookCreationOptions): Promise { - const kernelConnection = options.kernelConnection; - // Make sure we have a server - const serverOptions: GetServerOptions = isLocalConnection(kernelConnection) - ? { - ui: options.ui, - resource: options.resource, - token: options.token, - localJupyter: true - } - : { - ui: options.ui, - resource: options.resource, - token: options.token, - localJupyter: false, - serverId: kernelConnection.serverId - }; - const server = await this.serverProvider.getOrCreateServer(serverOptions); - Cancellation.throwIfCanceled(options.token); - return server.createNotebook( - options.resource, - options.kernelConnection, - options.token, - options.ui, - options.creator - ); - } -} diff --git a/src/kernels/jupyter/launcher/jupyterServerConnector.ts b/src/kernels/jupyter/launcher/jupyterServerConnector.ts index c7352660a46..950f3cb2db0 100644 --- a/src/kernels/jupyter/launcher/jupyterServerConnector.ts +++ b/src/kernels/jupyter/launcher/jupyterServerConnector.ts @@ -6,7 +6,7 @@ import { IPythonExtensionChecker } from '../../../platform/api/types'; import { ConnectNotebookProviderOptions, IJupyterConnection, IJupyterServerConnector } from '../../types'; import { DisplayOptions } from '../../displayOptions'; import { IRawKernelConnectionSessionCreator } from '../../raw/types'; -import { IJupyterNotebookProvider } from '../types'; +import { IJupyterServerProvider } from '../types'; import { PythonExtensionNotInstalledError } from '../../../platform/errors/pythonExtNotInstalledError'; @injectable() @@ -16,8 +16,8 @@ export class JupyterServerConnector implements IJupyterServerConnector { @inject(IRawKernelConnectionSessionCreator) @optional() private readonly rawSessionCreator: IRawKernelConnectionSessionCreator | undefined, - @inject(IJupyterNotebookProvider) - private readonly jupyterNotebookProvider: IJupyterNotebookProvider, + @inject(IJupyterServerProvider) + private readonly jupyterNotebookProvider: IJupyterServerProvider, @inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker ) {} @@ -32,13 +32,13 @@ export class JupyterServerConnector implements IJupyterServerConnector { } }); options.ui = this.startupUi; - if (this.rawSessionCreator?.isSupported && options.localJupyter) { + if (this.rawSessionCreator?.isSupported) { throw new Error('Connect method should not be invoked for local Connections when Raw is supported'); - } else if (this.extensionChecker.isPythonExtensionInstalled || !options.localJupyter) { - return this.jupyterNotebookProvider.connect(options).finally(() => handler.dispose()); + } else if (this.extensionChecker.isPythonExtensionInstalled) { + return this.jupyterNotebookProvider.getOrCreateServer(options).finally(() => handler.dispose()); } else { handler.dispose(); - if (!this.startupUi.disableUI && options.localJupyter) { + if (!this.startupUi.disableUI) { await this.extensionChecker.showPythonExtensionInstallRequiredPrompt(); } throw new PythonExtensionNotInstalledError(); diff --git a/src/kernels/jupyter/launcher/kernelConnectionSessionCreator.ts b/src/kernels/jupyter/launcher/kernelConnectionSessionCreator.ts deleted file mode 100644 index 1d92034c1f3..00000000000 --- a/src/kernels/jupyter/launcher/kernelConnectionSessionCreator.ts +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { inject, injectable, optional } from 'inversify'; -import { - GetServerOptions, - IKernelConnectionSession, - IKernelConnectionSessionCreator, - isLocalConnection, - NotebookCreationOptions -} from '../../types'; -import { Cancellation } from '../../../platform/common/cancellation'; -import { IRawKernelConnectionSessionCreator } from '../../raw/types'; -import { IJupyterNotebookProvider } from '../types'; - -/** - * Generic class for connecting to a server. Probably could be renamed as it doesn't provide notebooks, but rather connections. - */ -@injectable() -export class KernelConnectionSessionCreator implements IKernelConnectionSessionCreator { - constructor( - @inject(IRawKernelConnectionSessionCreator) - @optional() - private readonly rawKernelSessionCreator: IRawKernelConnectionSessionCreator | undefined, - @inject(IJupyterNotebookProvider) - private readonly jupyterNotebookProvider: IJupyterNotebookProvider - ) {} - - public async create(options: NotebookCreationOptions): Promise { - const kernelConnection = options.kernelConnection; - const isLocal = isLocalConnection(kernelConnection); - - if (this.rawKernelSessionCreator?.isSupported && isLocal) { - return this.rawKernelSessionCreator.create( - options.resource, - options.kernelConnection, - options.ui, - options.token - ); - } - const serverOptions: GetServerOptions = isLocal - ? { - resource: options.resource, - token: options.token, - ui: options.ui, - localJupyter: true - } - : { - resource: options.resource, - token: options.token, - ui: options.ui, - localJupyter: false, - serverId: kernelConnection.serverId - }; - await this.jupyterNotebookProvider.connect(serverOptions); - Cancellation.throwIfCanceled(options.token); - - return this.jupyterNotebookProvider.createNotebook(options); - } -} diff --git a/src/kernels/jupyter/launcher/liveshare/hostJupyterExecution.ts b/src/kernels/jupyter/launcher/liveshare/hostJupyterExecution.ts deleted file mode 100644 index 1d0c89d8fe6..00000000000 --- a/src/kernels/jupyter/launcher/liveshare/hostJupyterExecution.ts +++ /dev/null @@ -1,287 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import uuid from 'uuid/v4'; -import { CancellationToken, Uri } from 'vscode'; -import { ServerCache } from './serverCache'; -import { inject, injectable, optional } from 'inversify'; -import { IWorkspaceService } from '../../../../platform/common/application/types'; -import { traceInfo, traceVerbose, traceWarning } from '../../../../platform/logging'; -import { - IDisposableRegistry, - IAsyncDisposableRegistry, - IConfigurationService -} from '../../../../platform/common/types'; -import { testOnlyMethod } from '../../../../platform/common/utils/decorators'; -import { IInterpreterService } from '../../../../platform/interpreter/contracts'; -import { - IJupyterExecution, - INotebookServerOptions, - INotebookServer, - INotebookStarter, - INotebookServerFactory, - IJupyterServerUriStorage -} from '../../types'; -import * as urlPath from '../../../../platform/vscode-path/resources'; -import { IJupyterSubCommandExecutionService } from '../../types.node'; -import { JupyterConnection } from '../../connection/jupyterConnection'; -import { PythonEnvironment } from '../../../../platform/pythonEnvironments/info'; -import { DataScience } from '../../../../platform/common/utils/localize'; -import { Cancellation } from '../../../../platform/common/cancellation'; -import { IJupyterConnection } from '../../../types'; -import { JupyterSelfCertsError } from '../../../../platform/errors/jupyterSelfCertsError'; -import { JupyterSelfCertsExpiredError } from '../../../../platform/errors/jupyterSelfCertsExpiredError'; -import { LocalJupyterServerConnectionError } from '../../../../platform/errors/localJupyterServerConnectionError'; -import { RemoteJupyterServerConnectionError } from '../../../../platform/errors/remoteJupyterServerConnectionError'; -import { sendTelemetryEvent, Telemetry } from '../../../../telemetry'; -import { JupyterWaitForIdleError } from '../../../errors/jupyterWaitForIdleError'; -import { expandWorkingDir } from '../../jupyterUtils'; - -/* eslint-disable @typescript-eslint/no-explicit-any */ -const LocalHosts = ['localhost', '127.0.0.1', '::1']; - -/** - * Jupyter server implementation that uses the JupyterExecutionBase class to launch Jupyter. - */ -@injectable() -export class HostJupyterExecution implements IJupyterExecution { - private usablePythonInterpreter: PythonEnvironment | undefined; - private disposed: boolean = false; - private serverCache: ServerCache; - private _disposed = false; - private _id = uuid(); - constructor( - @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, - @inject(IDisposableRegistry) private readonly disposableRegistry: IDisposableRegistry, - @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, - @inject(IWorkspaceService) private readonly workspace: IWorkspaceService, - @inject(IConfigurationService) private readonly configuration: IConfigurationService, - @inject(INotebookStarter) @optional() private readonly notebookStarter: INotebookStarter | undefined, - @inject(IJupyterSubCommandExecutionService) - @optional() - private readonly jupyterInterpreterService: IJupyterSubCommandExecutionService | undefined, - @inject(INotebookServerFactory) private readonly notebookServerFactory: INotebookServerFactory, - @inject(IJupyterServerUriStorage) private readonly serverUriStorage: IJupyterServerUriStorage, - @inject(JupyterConnection) private readonly jupyterConnection: JupyterConnection - ) { - this.disposableRegistry.push(this.interpreterService.onDidChangeInterpreter(() => this.onSettingsChanged())); - this.disposableRegistry.push(this); - - if (workspace) { - workspace.onDidChangeConfiguration( - (e) => { - if (e.affectsConfiguration('python.dataScience', undefined)) { - // When config changes happen, recreate our commands. - this.onSettingsChanged(); - } - }, - this, - this.disposableRegistry - ); - } - this.serverCache = new ServerCache(); - this.serverUriStorage.onDidChangeUri( - () => { - this.serverCache.clearCache(); - }, - this, - disposableRegistry - ); - asyncRegistry.push(this); - } - - @testOnlyMethod() - public clearCache() { - this.serverCache.clearCache(); - } - public async dispose(): Promise { - traceInfo(`Disposing HostJupyterExecution ${this._id}`); - if (!this._disposed) { - this._disposed = true; - traceVerbose(`Disposing super HostJupyterExecution ${this._id}`); - this.disposed = true; - - // Cleanup on dispose. We are going away permanently - if (this.serverCache) { - traceVerbose(`Cleaning up server cache ${this._id}`); - await this.serverCache.dispose(); - } - } - traceVerbose(`Finished disposing HostJupyterExecution ${this._id}`); - } - - private async hostConnectToNotebookServer( - options: INotebookServerOptions, - cancelToken: CancellationToken - ): Promise { - if (!this._disposed) { - return this.connectToNotebookServerImpl( - await this.serverCache.generateDefaultOptions(options), - cancelToken - ); - } - throw new Error('Notebook server is disposed'); - } - - public async connectToNotebookServer( - options: INotebookServerOptions, - cancelToken: CancellationToken - ): Promise { - if (!this._disposed) { - return this.serverCache.getOrCreate(this.hostConnectToNotebookServer.bind(this), options, cancelToken); - } - throw new Error('Notebook server is disposed'); - } - public async getServer(options: INotebookServerOptions): Promise { - if (!this._disposed) { - // See if we have this server or not. - return this.serverCache.get(options); - } - } - - public async refreshCommands(): Promise { - await this.jupyterInterpreterService?.refreshCommands(); - } - - public async isNotebookSupported(cancelToken?: CancellationToken): Promise { - // See if we can find the command notebook - return this.jupyterInterpreterService ? this.jupyterInterpreterService.isNotebookSupported(cancelToken) : false; - } - - public async getNotebookError(): Promise { - return this.jupyterInterpreterService - ? this.jupyterInterpreterService.getReasonForJupyterNotebookNotBeingSupported() - : DataScience.webNotSupported; - } - - public async getUsableJupyterPython(cancelToken?: CancellationToken): Promise { - // Only try to compute this once. - if (!this.usablePythonInterpreter && !this.disposed && this.jupyterInterpreterService) { - this.usablePythonInterpreter = await Cancellation.race( - () => this.jupyterInterpreterService!.getSelectedInterpreter(cancelToken), - cancelToken - ); - } - return this.usablePythonInterpreter; - } - - /* eslint-disable complexity, */ - public connectToNotebookServerImpl( - options: INotebookServerOptions, - cancelToken: CancellationToken - ): Promise { - // Return nothing if we cancel - // eslint-disable-next-line - return Cancellation.race(async () => { - let result: INotebookServer | undefined; - let connection: IJupyterConnection | undefined; - - // Try to connect to our jupyter process. Check our setting for the number of tries - let tryCount = 1; - const maxTries = Math.max(1, this.configuration.getSettings(undefined).jupyterLaunchRetries); - let lastTryError: Error; - while (tryCount <= maxTries && !this.disposed) { - try { - // Start or connect to the process - connection = await this.startOrConnect(options, cancelToken); - - if (!connection.localLaunch && LocalHosts.includes(connection.hostName.toLowerCase())) { - sendTelemetryEvent(Telemetry.ConnectRemoteJupyterViaLocalHost); - } - // eslint-disable-next-line no-constant-condition - traceVerbose(`Connecting to process server`); - - // Create a server tha t we will then attempt to connect to. - result = await this.notebookServerFactory.createNotebookServer(connection); - traceVerbose(`Connection complete server`); - return result; - } catch (err) { - lastTryError = err; - // Cleanup after ourselves. server may be running partially. - if (result) { - traceWarning(`Killing server because of error ${err}`); - await result.dispose(); - } - if (err instanceof JupyterWaitForIdleError && tryCount < maxTries) { - // Special case. This sometimes happens where jupyter doesn't ever connect. Cleanup after - // ourselves and propagate the failure outwards. - traceInfo('Retry because of wait for idle problem.'); - - // Close existing connection. - connection?.dispose(); - tryCount += 1; - } else if (connection) { - // If this is occurring during shutdown, don't worry about it. - if (this.disposed) { - throw err; - } - - // Something else went wrong - if (!options.localJupyter) { - sendTelemetryEvent(Telemetry.ConnectRemoteFailedJupyter, undefined, undefined, err); - - // Check for the self signed certs error specifically - if (JupyterSelfCertsError.isSelfCertsError(err)) { - sendTelemetryEvent(Telemetry.ConnectRemoteSelfCertFailedJupyter); - throw new JupyterSelfCertsError(connection.baseUrl); - } else if (JupyterSelfCertsExpiredError.isSelfCertsExpiredError(err)) { - sendTelemetryEvent(Telemetry.ConnectRemoteExpiredCertFailedJupyter); - throw new JupyterSelfCertsExpiredError(connection.baseUrl); - } else { - throw new RemoteJupyterServerConnectionError(connection.baseUrl, options.serverId, err); - } - } else { - sendTelemetryEvent(Telemetry.ConnectFailedJupyter, undefined, undefined, err); - throw new LocalJupyterServerConnectionError(err); - } - } else { - throw err; - } - } - throw lastTryError; - } - throw new Error('Max number of attempts reached'); - }, cancelToken); - } - - private async startOrConnect( - options: INotebookServerOptions, - cancelToken: CancellationToken - ): Promise { - // If our uri is undefined or if it's set to local launch we need to launch a server locally - if (options.localJupyter) { - // If that works, then attempt to start the server - traceInfo(`Launching server`); - const settings = this.configuration.getSettings(options.resource); - const useDefaultConfig = settings.useDefaultConfigForJupyter; - const workingDir = await this.workspace.computeWorkingDirectory(options.resource); - // Expand the working directory. Create a dummy launching file in the root path (so we expand correctly) - const workingDirectory = expandWorkingDir( - workingDir, - this.workspace.rootFolder ? urlPath.joinPath(this.workspace.rootFolder, `${uuid()}.txt`) : undefined, - this.workspace, - settings - ); - - if (!this.notebookStarter) { - // In desktop mode this must be defined, in web this code path never gets executed. - throw new Error('Notebook Starter cannot be undefined'); - } - return this.notebookStarter!.start( - options.resource, - useDefaultConfig, - this.configuration.getSettings(undefined).jupyterCommandLineArguments, - Uri.file(workingDirectory), - cancelToken - ); - } else { - // If we have a URI spec up a connection info for it - return this.jupyterConnection.createConnectionInfo({ serverId: options.serverId }); - } - } - - private onSettingsChanged() { - // Clear our usableJupyterInterpreter so that we recompute our values - this.usablePythonInterpreter = undefined; - } -} diff --git a/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts b/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts deleted file mode 100644 index cb746ff1737..00000000000 --- a/src/kernels/jupyter/launcher/liveshare/hostJupyterServer.ts +++ /dev/null @@ -1,247 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { CancellationToken } from 'vscode-jsonrpc'; -import { inject } from 'inversify'; -import { IWorkspaceService } from '../../../../platform/common/application/types'; -import { traceInfo, traceError, traceInfoIfCI, traceVerbose } from '../../../../platform/logging'; -import { - IAsyncDisposableRegistry, - IDisposableRegistry, - Resource, - IDisposable, - IDisplayOptions -} from '../../../../platform/common/types'; -import { createDeferred, sleep } from '../../../../platform/common/utils/async'; -import { DataScience } from '../../../../platform/common/utils/localize'; -import { SessionDisposedError } from '../../../../platform/errors/sessionDisposedError'; -import { - KernelConnectionMetadata, - isLocalConnection, - IJupyterConnection, - KernelActionSource, - IJupyterKernelConnectionSession -} from '../../../types'; -import { JupyterSessionManager } from '../../session/jupyterSessionManager'; -import { noop } from '../../../../platform/common/utils/misc'; -import { Cancellation } from '../../../../platform/common/cancellation'; -import { getDisplayPath } from '../../../../platform/common/platform/fs-paths'; -import { INotebookServer } from '../../types'; -import { Uri } from 'vscode'; -import { RemoteJupyterServerConnectionError } from '../../../../platform/errors/remoteJupyterServerConnectionError'; -/* eslint-disable @typescript-eslint/no-explicit-any */ - -/** - * Represents a connection to a Jupyter server. - */ -export class HostJupyterServer implements INotebookServer { - private connectionInfoDisconnectHandler: IDisposable | undefined; - private serverExitCode: number | undefined; - private sessions = new Set>(); - private disposed = false; - constructor( - @inject(IAsyncDisposableRegistry) private readonly asyncRegistry: IAsyncDisposableRegistry, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, - public connection: IJupyterConnection, - private sessionManager: JupyterSessionManager - ) { - this.asyncRegistry.push(this); - - this.connectionInfoDisconnectHandler = this.connection.disconnected((c) => { - try { - this.serverExitCode = c; - traceError(DataScience.jupyterServerCrashed(c)); - this.shutdown().catch(noop); - } catch { - noop(); - } - }); - } - - public async dispose(): Promise { - if (!this.disposed) { - this.disposed = true; - traceVerbose(`Disposing HostJupyterServer`); - await this.shutdown(); - traceVerbose(`Finished disposing HostJupyterServer`); - } - } - - private get isDisposed() { - return this.disposed; - } - private throwIfDisposedOrCancelled(token?: CancellationToken) { - if (this.isDisposed) { - throw new SessionDisposedError(); - } - Cancellation.throwIfCanceled(token); - } - private async createNotebookInstance( - resource: Resource, - sessionManager: JupyterSessionManager, - kernelConnection: KernelConnectionMetadata, - cancelToken: CancellationToken, - ui: IDisplayOptions, - actionSource: KernelActionSource - ): Promise { - this.throwIfDisposedOrCancelled(cancelToken); - // Compute launch information from the resource and the notebook metadata - const sessionPromise = createDeferred(); - // Save the Session - this.trackDisposable(sessionPromise.promise); - const startNewSession = async () => { - this.throwIfDisposedOrCancelled(cancelToken); - // Figure out the working directory we need for our new notebook. This is only necessary for local. - const workingDirectory = isLocalConnection(kernelConnection) - ? await this.workspaceService.computeWorkingDirectory(resource) - : ''; - this.throwIfDisposedOrCancelled(cancelToken); - // Start a session (or use the existing one if allowed) - const session = await sessionManager.startNew( - resource, - kernelConnection, - Uri.file(workingDirectory), - ui, - cancelToken, - actionSource - ); - this.throwIfDisposedOrCancelled(cancelToken); - traceInfo(`Started session for kernel ${kernelConnection.kind}:${kernelConnection.id}`); - return session; - }; - - try { - const session = await startNewSession(); - this.throwIfDisposedOrCancelled(cancelToken); - - if (session) { - sessionPromise.resolve(session); - } else { - sessionPromise.reject(this.getDisposedError()); - } - } catch (ex) { - // If there's an error, then reject the promise that is returned. - // This original promise must be rejected as it is cached (check `setNotebook`). - sessionPromise.reject(ex); - } - - return sessionPromise.promise; - } - - public async createNotebook( - resource: Resource, - kernelConnection: KernelConnectionMetadata, - cancelToken: CancellationToken, - ui: IDisplayOptions, - creator: KernelActionSource - ): Promise { - this.throwIfDisposedOrCancelled(cancelToken); - traceInfoIfCI( - `HostJupyterServer.createNotebook for ${getDisplayPath(resource)} with ui.disableUI=${ - ui.disableUI - }, cancelToken.isCancellationRequested=${cancelToken.isCancellationRequested}` - ); - if (!this.sessionManager || this.isDisposed) { - throw new SessionDisposedError(); - } - if ( - this.sessionManager && - !this.isDisposed && - (kernelConnection.kind === 'connectToLiveRemoteKernel' || - kernelConnection.kind === 'startUsingRemoteKernelSpec') - ) { - try { - await Promise.all([this.sessionManager.getRunningKernels(), this.sessionManager.getKernelSpecs()]); - } catch (ex) { - traceError( - 'Failed to fetch running kernels from remote server, connection may be outdated or remote server may be unreachable', - ex - ); - throw new RemoteJupyterServerConnectionError(kernelConnection.baseUrl, kernelConnection.serverId, ex); - } - } - // Create a session and return it. - const session = await this.createNotebookInstance( - resource, - this.sessionManager, - kernelConnection, - cancelToken, - ui, - creator - ); - this.throwIfDisposedOrCancelled(cancelToken); - const baseUrl = this.connection?.baseUrl || ''; - traceVerbose(DataScience.createdNewNotebook(baseUrl)); - return session; - } - - private async shutdown(): Promise { - try { - // Order should be - // 1) connectionInfoDisconnectHandler - listens to process close - // 2) sessions (owned by the notebooks) - // 3) session manager (owned by this object) - // 4) connInfo (owned by this object) - kills the jupyter process - - if (this.connectionInfoDisconnectHandler) { - this.connectionInfoDisconnectHandler.dispose(); - this.connectionInfoDisconnectHandler = undefined; - } - - traceVerbose('Shutting down notebooks'); - const session = await Promise.all([...this.sessions.values()]); - await Promise.all(session.map((session) => session.dispose())); - traceVerbose(`Shut down session manager : ${this.sessionManager ? 'existing' : 'undefined'}`); - if (this.sessionManager) { - // Session manager in remote case may take too long to shutdown. Don't wait that - // long. - const result = await Promise.race([sleep(10_000), this.sessionManager.dispose()]); - if (result === 10_000) { - traceError(`Session shutdown timed out.`); - } - } - - // After shutting down notebooks and session manager, kill the main process. - if (this.connection && this.connection) { - traceVerbose('Shutdown server - dispose conn info'); - this.connection.dispose(); // This should kill the process that's running - } - } catch (e) { - traceError(`Error during shutdown: `, e); - } - } - - // Return a copy of the connection information that this server used to connect with - public getConnectionInfo(): IJupyterConnection { - if (!this.connection) { - throw new Error('Not connected'); - } - - // Return a copy with a no-op for dispose - return { - ...this.connection, - dispose: noop - }; - } - - public getDisposedError(): Error { - // We may have been disposed because of a crash. See if our connection info is indicating shutdown - if (this.serverExitCode) { - return new Error(DataScience.jupyterServerCrashed(this.serverExitCode)); - } - - // Default is just say session was disposed - return new SessionDisposedError(); - } - private trackDisposable(sessionPromise: Promise) { - sessionPromise - .then((session) => { - session.onDidDispose(() => this.sessions.delete(sessionPromise), this, this.disposables); - }) - .catch(() => this.sessions.delete(sessionPromise)); - - // Save the notebook - this.sessions.add(sessionPromise); - } -} diff --git a/src/kernels/jupyter/launcher/liveshare/hostJupyterServerFactory.ts b/src/kernels/jupyter/launcher/liveshare/hostJupyterServerFactory.ts deleted file mode 100644 index 5558276f3eb..00000000000 --- a/src/kernels/jupyter/launcher/liveshare/hostJupyterServerFactory.ts +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { inject, injectable } from 'inversify'; -import {} from 'underscore'; -import { IWorkspaceService } from '../../../../platform/common/application/types'; -import { IAsyncDisposableRegistry, IDisposableRegistry } from '../../../../platform/common/types'; -import { traceInfo } from '../../../../platform/logging'; -import { IJupyterConnection } from '../../../types'; -import { JupyterSessionManager } from '../../session/jupyterSessionManager'; -import { IJupyterSessionManagerFactory, INotebookServer, INotebookServerFactory } from '../../types'; -import { HostJupyterServer } from './hostJupyterServer'; - -/** - * Factory for HostJupyterServer. - */ -@injectable() -export class HostJupyterServerFactory implements INotebookServerFactory { - constructor( - @inject(IAsyncDisposableRegistry) private readonly asyncRegistry: IAsyncDisposableRegistry, - @inject(IJupyterSessionManagerFactory) private readonly sessionManagerFactory: IJupyterSessionManagerFactory, - @inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry - ) {} - public async createNotebookServer(connection: IJupyterConnection): Promise { - traceInfo(`Connecting ${connection.localLaunch ? 'local' : 'remote'} server kernel ${connection.baseUrl}`); - - // Create our session manager - const sessionManager = (await this.sessionManagerFactory.create(connection)) as JupyterSessionManager; - // Create a server tha t we will then attempt to connect to. - return new HostJupyterServer( - this.asyncRegistry, - this.workspaceService, - this.disposables, - connection, - sessionManager - ); - } -} diff --git a/src/kernels/jupyter/launcher/liveshare/serverCache.ts b/src/kernels/jupyter/launcher/liveshare/serverCache.ts deleted file mode 100644 index e2ac85616d6..00000000000 --- a/src/kernels/jupyter/launcher/liveshare/serverCache.ts +++ /dev/null @@ -1,124 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { CancellationToken } from 'vscode'; -import { traceError, traceVerbose } from '../../../../platform/logging'; -import { IAsyncDisposable } from '../../../../platform/common/types'; -import { sleep } from '../../../../platform/common/utils/async'; -import { INotebookServerOptions, INotebookServer } from '../../types'; - -interface IServerData { - options: INotebookServerOptions; - promise: Promise; - resolved: boolean; -} - -/** - * Cache of connections to notebook servers. - */ -export class ServerCache implements IAsyncDisposable { - private cache: Map = new Map(); - private disposed = false; - - public clearCache() { - this.cache.clear(); - } - - public async getOrCreate( - createFunction: (options: INotebookServerOptions, cancelToken: CancellationToken) => Promise, - options: INotebookServerOptions, - cancelToken: CancellationToken - ): Promise { - const fixedOptions = await this.generateDefaultOptions(options); - const key = this.generateKey(fixedOptions); - let data: IServerData | undefined; - - // Check to see if we already have a promise for this key - data = this.cache.get(key); - - if (!data) { - // Didn't find one, so start up our promise and cache it - data = { - promise: createFunction(options, cancelToken), - options: fixedOptions, - resolved: false - }; - this.cache.set(key, data); - } - - return data.promise - .then((server: INotebookServer) => { - // Change the dispose on it so we - // can detach from the server when it goes away. - const oldDispose = server.dispose.bind(server); - server.dispose = () => { - this.cache.delete(key); - return oldDispose(); - }; - - // We've resolved the promise at this point - if (data) { - data.resolved = true; - } - - return server; - }) - .catch((e) => { - this.cache.delete(key); - throw e; - }); - } - - public async get(options: INotebookServerOptions): Promise { - const fixedOptions = await this.generateDefaultOptions(options); - const key = this.generateKey(fixedOptions); - if (this.cache.has(key)) { - return this.cache.get(key)?.promise; - } - } - - public async dispose(): Promise { - if (!this.disposed) { - this.disposed = true; - const entries = [...this.cache.values()]; - this.cache.clear(); - await Promise.all( - entries.map(async (d) => { - try { - // This should be quick. The server is either already up or will never come back. - const server = await Promise.race([d.promise, sleep(1000)]); - if (typeof server !== 'number') { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - await (server as any).dispose(); - } else { - traceVerbose('ServerCache Dispose, no server'); - } - } catch (e) { - traceError(`Dispose error in ServerCache: `, e); - } - }) - ); - } - } - - public async generateDefaultOptions(options: INotebookServerOptions): Promise { - if (options.localJupyter) { - return { - resource: options?.resource, - ui: options.ui, - localJupyter: true - }; - } - return { - serverId: options.serverId, - resource: options?.resource, - ui: options.ui, - localJupyter: false - }; - } - - private generateKey(options: INotebookServerOptions): string { - // combine all the values together to make a unique key - return `serverId=${options.localJupyter ? '' : options.serverId};local=${options.localJupyter};`; - } -} diff --git a/src/kernels/jupyter/launcher/notebookServerProvider.ts b/src/kernels/jupyter/launcher/notebookServerProvider.ts index e8f37b85689..d553a3542ba 100644 --- a/src/kernels/jupyter/launcher/notebookServerProvider.ts +++ b/src/kernels/jupyter/launcher/notebookServerProvider.ts @@ -2,65 +2,27 @@ // Licensed under the MIT License. import { inject, injectable, optional } from 'inversify'; -import { disposeAllDisposables } from '../../../platform/common/helpers'; import { traceVerbose } from '../../../platform/logging'; -import { IDisposable, IDisposableRegistry } from '../../../platform/common/types'; -import { testOnlyMethod } from '../../../platform/common/utils/decorators'; import { DataScience } from '../../../platform/common/utils/localize'; import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { JupyterInstallError } from '../../../platform/errors/jupyterInstallError'; -import { KernelProgressReporter } from '../../../platform/progress/kernelProgressReporter'; -import { DisplayOptions } from '../../displayOptions'; -import { GetServerOptions } from '../../types'; -import { - IJupyterServerProvider, - INotebookServer, - IJupyterExecution, - IJupyterServerUriStorage, - INotebookServerOptions -} from '../types'; +import { GetServerOptions, IJupyterConnection } from '../../types'; +import { IJupyterServerProvider, IJupyterExecution } from '../types'; import { NotSupportedInWebError } from '../../../platform/errors/notSupportedInWebError'; import { getFilePath } from '../../../platform/common/platform/fs-paths'; -import { isCancellationError } from '../../../platform/common/cancellation'; +import { Cancellation, isCancellationError } from '../../../platform/common/cancellation'; -/** - * Starts jupyter servers locally. - */ -const localCacheKey = 'LocalJupyterSererCacheKey'; @injectable() export class NotebookServerProvider implements IJupyterServerProvider { - private serverPromise = new Map>(); - private ui = new DisplayOptions(true); + private serverPromise?: Promise; constructor( @inject(IJupyterExecution) @optional() private readonly jupyterExecution: IJupyterExecution | undefined, - @inject(IInterpreterService) private readonly interpreterService: IInterpreterService, - @inject(IJupyterServerUriStorage) serverUriStorage: IJupyterServerUriStorage, - @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry - ) { - serverUriStorage.onDidChangeUri( - () => { - // Possible user selected another Server. - const localCache = this.serverPromise.get('local'); - this.serverPromise.clear(); - // Restore the cache for local servers. - if (localCache) { - this.serverPromise.set(localCacheKey, localCache); - } - }, - this, - this.disposables - ); - } - @testOnlyMethod() - public clearCache() { - this.serverPromise.clear(); - } - public async getOrCreateServer(options: GetServerOptions): Promise { - const serverOptions = this.getNotebookServerOptions(options); - + @inject(IInterpreterService) private readonly interpreterService: IInterpreterService + ) {} + public async getOrCreateServer(options: GetServerOptions): Promise { // If we are just fetching or only want to create for local, see if exists - if (options.localJupyter && this.jupyterExecution) { - const server = await this.jupyterExecution.getServer(serverOptions); + if (this.jupyterExecution) { + const server = await this.jupyterExecution.getServer(options.resource); // Possible it wasn't created, hence create it. if (server) { return server; @@ -71,60 +33,30 @@ export class NotebookServerProvider implements IJupyterServerProvider { return this.createServer(options); } - private async createServer(options: GetServerOptions): Promise { - // When we finally try to create a server, update our flag indicating if we're going to allow UI or not. This - // allows the server to be attempted without a UI, but a future request can come in and use the same startup - if (!options.ui.disableUI) { - this.ui.disableUI = false; - } - options.ui.onDidChangeDisableUI(() => (this.ui.disableUI = options.ui.disableUI), this, this.disposables); - const cacheKey = options.localJupyter ? localCacheKey : options.serverId; - if (!this.serverPromise.has(cacheKey)) { + private async createServer(options: GetServerOptions): Promise { + if (!this.serverPromise) { // Start a server - this.serverPromise.set(cacheKey, this.startServer(options)); - } - try { - const value = await this.serverPromise.get(cacheKey)!; - // If we cancelled starting of the server, then don't cache the result. - if (!value && options.token?.isCancellationRequested) { - this.serverPromise.delete(cacheKey); - } - return value; - } catch (e) { - // Don't cache the error - this.serverPromise.delete(cacheKey); - throw e; + const promise = (this.serverPromise = this.startServer(options)); + promise.catch(() => { + if (promise === this.serverPromise) { + this.serverPromise = undefined; + } + }); } + return this.serverPromise; } - private async startServer(options: GetServerOptions): Promise { + private async startServer(options: GetServerOptions): Promise { const jupyterExecution = this.jupyterExecution; if (!jupyterExecution) { throw new NotSupportedInWebError(); } - const serverOptions = this.getNotebookServerOptions(options); - const disposables: IDisposable[] = []; - let progressReporter: IDisposable | undefined; - const createProgressReporter = async () => { - if (this.ui.disableUI || progressReporter) { - return; - } - // Status depends upon if we're about to connect to existing server or not. - progressReporter = !serverOptions.localJupyter - ? KernelProgressReporter.createProgressReporter(options.resource, DataScience.connectingToJupyter) - : KernelProgressReporter.createProgressReporter(options.resource, DataScience.startingJupyter); - disposables.push(progressReporter); - }; - if (this.ui.disableUI) { - this.ui.onDidChangeDisableUI(createProgressReporter, this, disposables); - } // Check to see if we support ipykernel or not try { - await createProgressReporter(); traceVerbose(`Checking for server usability.`); - const usable = await this.checkUsable(serverOptions); + const usable = await this.checkUsable(); if (!usable) { traceVerbose('Server not usable (should ask for install now)'); // Indicate failing. @@ -134,9 +66,10 @@ export class NotebookServerProvider implements IJupyterServerProvider { } // Then actually start the server traceVerbose(`Starting notebook server.`); - return await jupyterExecution.connectToNotebookServer(serverOptions, options.token); + const result = await jupyterExecution.connectToNotebookServer(options.resource, options.token); + Cancellation.throwIfCanceled(options.token); + return result; } catch (e) { - disposeAllDisposables(disposables); // If user cancelled, then do nothing. if (options.token?.isCancellationRequested && isCancellationError(e)) { throw e; @@ -147,14 +80,12 @@ export class NotebookServerProvider implements IJupyterServerProvider { await jupyterExecution.refreshCommands(); throw e; - } finally { - disposeAllDisposables(disposables); } } - private async checkUsable(options: INotebookServerOptions): Promise { + private async checkUsable(): Promise { try { - if (options.localJupyter && this.jupyterExecution) { + if (this.jupyterExecution) { const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython(); return usableInterpreter ? true : false; } else { @@ -177,21 +108,4 @@ export class NotebookServerProvider implements IJupyterServerProvider { } } } - - private getNotebookServerOptions(options: GetServerOptions): INotebookServerOptions { - if (options.localJupyter) { - return { - resource: options.resource, - ui: this.ui, - localJupyter: true - }; - } - - return { - serverId: options.serverId, - resource: options.resource, - ui: this.ui, - localJupyter: false - }; - } } diff --git a/src/kernels/jupyter/launcher/notebookServerProvider.unit.test.ts b/src/kernels/jupyter/launcher/notebookServerProvider.unit.test.ts index f0ed66cda46..807a9ac1b0f 100644 --- a/src/kernels/jupyter/launcher/notebookServerProvider.unit.test.ts +++ b/src/kernels/jupyter/launcher/notebookServerProvider.unit.test.ts @@ -10,9 +10,9 @@ import { disposeAllDisposables } from '../../../platform/common/helpers'; import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { PythonEnvironment } from '../../../platform/pythonEnvironments/info'; import { NotebookServerProvider } from '../../../kernels/jupyter/launcher/notebookServerProvider'; -import { JupyterServerUriStorage } from '../../../kernels/jupyter/connection/serverUriStorage'; import { DisplayOptions } from '../../../kernels/displayOptions'; -import { IJupyterExecution, INotebookServer } from '../../../kernels/jupyter/types'; +import { IJupyterExecution } from '../../../kernels/jupyter/types'; +import { IJupyterConnection } from '../../types'; /* eslint-disable @typescript-eslint/no-explicit-any */ function createTypeMoq(tag: string): typemoq.IMock { @@ -42,36 +42,25 @@ suite('NotebookServerProvider', () => { jupyterExecution = mock(); interpreterService = mock(); - const serverStorage = mock(JupyterServerUriStorage); - when(serverStorage.getUri()).thenResolve({ uri: 'local', time: Date.now(), serverId: 'local' }); - when(serverStorage.getRemoteUri()).thenResolve(); const eventEmitter = new EventEmitter(); disposables.push(eventEmitter); - when(serverStorage.onDidChangeUri).thenReturn(eventEmitter.event); when((jupyterExecution as any).then).thenReturn(undefined); - when((serverStorage as any).then).thenReturn(undefined); // Create the server provider - serverProvider = new NotebookServerProvider( - instance(jupyterExecution), - instance(interpreterService), - instance(serverStorage), - disposables - ); + serverProvider = new NotebookServerProvider(instance(jupyterExecution), instance(interpreterService)); source = new CancellationTokenSource(); disposables.push(source); }); teardown(() => disposeAllDisposables(disposables)); test('NotebookServerProvider - Get Only - server', async () => { - const notebookServer = mock(); - when((notebookServer as any).then).thenReturn(undefined); - when(jupyterExecution.getServer(anything())).thenResolve(instance(notebookServer)); + const connection = mock(); + when((connection as any).then).thenReturn(undefined); + when(jupyterExecution.getServer(anything())).thenResolve(instance(connection)); const server = await serverProvider.getOrCreateServer({ resource: undefined, ui: new DisplayOptions(false), - token: source.token, - localJupyter: true + token: source.token }); expect(server).to.not.equal(undefined, 'Server expected to be defined'); verify(jupyterExecution.getServer(anything())).once(); @@ -79,15 +68,14 @@ suite('NotebookServerProvider', () => { test('NotebookServerProvider - Get Or Create', async () => { when(jupyterExecution.getUsableJupyterPython()).thenResolve(workingPython); - const notebookServer = createTypeMoq('jupyter server'); - when(jupyterExecution.connectToNotebookServer(anything(), anything())).thenResolve(notebookServer.object); + const connection = createTypeMoq('jupyter server'); + when(jupyterExecution.connectToNotebookServer(anything(), anything())).thenResolve(connection.object); // Disable UI just lets us skip mocking the progress reporter const server = await serverProvider.getOrCreateServer({ ui: new DisplayOptions(true), resource: undefined, - token: source.token, - localJupyter: true + token: source.token }); expect(server).to.not.equal(undefined, 'Server expected to be defined'); }); diff --git a/src/kernels/jupyter/launcher/serverPreload.node.ts b/src/kernels/jupyter/launcher/serverPreload.node.ts index 29a6d461cf1..b94373334e6 100644 --- a/src/kernels/jupyter/launcher/serverPreload.node.ts +++ b/src/kernels/jupyter/launcher/serverPreload.node.ts @@ -87,7 +87,6 @@ export class ServerPreload implements IExtensionSyncActivationService { await this.serverConnector.connect({ resource: undefined, ui, - localJupyter: true, token: source.token }); } diff --git a/src/kernels/jupyter/serviceRegistry.node.ts b/src/kernels/jupyter/serviceRegistry.node.ts index 3ea2e13ed5c..a456e571096 100644 --- a/src/kernels/jupyter/serviceRegistry.node.ts +++ b/src/kernels/jupyter/serviceRegistry.node.ts @@ -25,10 +25,8 @@ import { JupyterKernelService } from './session/jupyterKernelService.node'; import { JupyterRemoteCachedKernelValidator } from './connection/jupyterRemoteCachedKernelValidator'; import { JupyterUriProviderRegistration } from './connection/jupyterUriProviderRegistration'; import { JupyterCommandLineSelector } from './launcher/commandLineSelector'; -import { JupyterNotebookProvider } from './launcher/jupyterNotebookProvider'; import { JupyterPasswordConnect } from './connection/jupyterPasswordConnect'; -import { HostJupyterExecution } from './launcher/liveshare/hostJupyterExecution'; -import { HostJupyterServerFactory } from './launcher/liveshare/hostJupyterServerFactory'; +import { HostJupyterExecution } from './launcher/hostJupyterExecution'; import { JupyterServerConnector } from './launcher/jupyterServerConnector'; import { NotebookServerProvider } from './launcher/notebookServerProvider'; import { NotebookStarter } from './launcher/notebookStarter.node'; @@ -40,7 +38,6 @@ import { JupyterRequestCreator } from './session/jupyterRequestCreator.node'; import { JupyterSessionManagerFactory } from './session/jupyterSessionManagerFactory'; import { RequestAgentCreator } from './session/requestAgentCreator.node'; import { - IJupyterNotebookProvider, IJupyterExecution, IJupyterPasswordConnect, IJupyterSessionManagerFactory, @@ -55,18 +52,16 @@ import { INotebookStarter, IJupyterRequestCreator, IJupyterRequestAgentCreator, - INotebookServerFactory, ILiveRemoteKernelConnectionUsageTracker, IJupyterRemoteCachedKernelValidator } from './types'; import { IJupyterCommandFactory, IJupyterSubCommandExecutionService } from './types.node'; import { RemoteKernelFinderController } from './finder/remoteKernelFinderController'; -import { KernelConnectionSessionCreator } from './launcher/kernelConnectionSessionCreator'; +import { KernelConnectionSessionCreator } from '../common/kernelConnectionSessionCreator'; +import { JupyterKernelConnectionSessionCreator } from './session/jupyterKernelConnectionSessionCreator'; export function registerTypes(serviceManager: IServiceManager, _isDevMode: boolean) { serviceManager.add(IJupyterCommandFactory, JupyterCommandFactory); - serviceManager.add(INotebookServerFactory, HostJupyterServerFactory); - serviceManager.addSingleton(IJupyterNotebookProvider, JupyterNotebookProvider); serviceManager.addSingleton( IExtensionSyncActivationService, JupyterInterpreterSelectionCommand @@ -126,6 +121,10 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole IKernelConnectionSessionCreator, KernelConnectionSessionCreator ); + serviceManager.addSingleton( + JupyterKernelConnectionSessionCreator, + JupyterKernelConnectionSessionCreator + ); serviceManager.addSingleton(IJupyterBackingFileCreator, BackingFileCreator); serviceManager.addSingleton(IJupyterRequestCreator, JupyterRequestCreator); serviceManager.addSingleton(IJupyterRequestAgentCreator, RequestAgentCreator); diff --git a/src/kernels/jupyter/serviceRegistry.web.ts b/src/kernels/jupyter/serviceRegistry.web.ts index 58af060f840..28c17e7f05c 100644 --- a/src/kernels/jupyter/serviceRegistry.web.ts +++ b/src/kernels/jupyter/serviceRegistry.web.ts @@ -11,10 +11,8 @@ import { JupyterKernelService } from './session/jupyterKernelService.web'; import { JupyterRemoteCachedKernelValidator } from './connection/jupyterRemoteCachedKernelValidator'; import { JupyterUriProviderRegistration } from './connection/jupyterUriProviderRegistration'; import { JupyterCommandLineSelector } from './launcher/commandLineSelector'; -import { JupyterNotebookProvider } from './launcher/jupyterNotebookProvider'; import { JupyterPasswordConnect } from './connection/jupyterPasswordConnect'; -import { HostJupyterExecution } from './launcher/liveshare/hostJupyterExecution'; -import { HostJupyterServerFactory } from './launcher/liveshare/hostJupyterServerFactory'; +import { HostJupyterExecution } from './launcher/hostJupyterExecution'; import { JupyterServerConnector } from './launcher/jupyterServerConnector'; import { NotebookServerProvider } from './launcher/notebookServerProvider'; import { JupyterServerUriStorage } from './connection/serverUriStorage'; @@ -30,21 +28,18 @@ import { IJupyterServerUriStorage, IJupyterBackingFileCreator, IJupyterKernelService, - IJupyterNotebookProvider, IJupyterServerProvider, IJupyterExecution, IJupyterRequestCreator, - INotebookServerFactory, ILiveRemoteKernelConnectionUsageTracker, IJupyterRemoteCachedKernelValidator } from './types'; import { RemoteKernelFinderController } from './finder/remoteKernelFinderController'; -import { KernelConnectionSessionCreator } from './launcher/kernelConnectionSessionCreator'; +import { KernelConnectionSessionCreator } from '../common/kernelConnectionSessionCreator'; +import { JupyterKernelConnectionSessionCreator } from './session/jupyterKernelConnectionSessionCreator'; export function registerTypes(serviceManager: IServiceManager, _isDevMode: boolean) { - serviceManager.addSingleton(IJupyterNotebookProvider, JupyterNotebookProvider); serviceManager.addSingleton(IJupyterExecution, HostJupyterExecution); - serviceManager.add(INotebookServerFactory, HostJupyterServerFactory); serviceManager.addSingleton(IJupyterPasswordConnect, JupyterPasswordConnect); serviceManager.addSingleton( IJupyterSessionManagerFactory, @@ -62,6 +57,10 @@ export function registerTypes(serviceManager: IServiceManager, _isDevMode: boole IKernelConnectionSessionCreator, KernelConnectionSessionCreator ); + serviceManager.addSingleton( + JupyterKernelConnectionSessionCreator, + JupyterKernelConnectionSessionCreator + ); serviceManager.addSingleton(IJupyterBackingFileCreator, BackingFileCreator); serviceManager.addSingleton(JupyterCommandLineSelector, JupyterCommandLineSelector); serviceManager.addSingleton(IJupyterServerProvider, NotebookServerProvider); diff --git a/src/kernels/jupyter/session/jupyterKernelConnectionSessionCreator.ts b/src/kernels/jupyter/session/jupyterKernelConnectionSessionCreator.ts new file mode 100644 index 00000000000..5ad150a521b --- /dev/null +++ b/src/kernels/jupyter/session/jupyterKernelConnectionSessionCreator.ts @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { Uri } from 'vscode'; +import { Cancellation } from '../../../platform/common/cancellation'; +import { + IJupyterKernelConnectionSession, + KernelConnectionSessionCreationOptions, + isLocalConnection, + isRemoteConnection +} from '../../types'; +import { IJupyterSessionManager } from '../types'; +import { traceError, traceInfo } from '../../../platform/logging'; +import { IWorkspaceService } from '../../../platform/common/application/types'; +import { inject, injectable } from 'inversify'; +import { noop } from '../../../platform/common/utils/misc'; +import { SessionDisposedError } from '../../../platform/errors/sessionDisposedError'; +import { RemoteJupyterServerConnectionError } from '../../../platform/errors/remoteJupyterServerConnectionError'; + +export type JupyterKernelConnectionSessionCreationOptions = KernelConnectionSessionCreationOptions & { + sessionManager: IJupyterSessionManager; +}; + +@injectable() +export class JupyterKernelConnectionSessionCreator { + constructor(@inject(IWorkspaceService) private readonly workspaceService: IWorkspaceService) {} + public async create( + options: JupyterKernelConnectionSessionCreationOptions + ): Promise { + if (options.sessionManager.isDisposed) { + throw new SessionDisposedError(); + } + if (isRemoteConnection(options.kernelConnection)) { + try { + await Promise.all([ + options.sessionManager.getRunningKernels(), + options.sessionManager.getKernelSpecs() + ]); + } catch (ex) { + traceError( + 'Failed to fetch running kernels from remote server, connection may be outdated or remote server may be unreachable', + ex + ); + throw new RemoteJupyterServerConnectionError( + options.kernelConnection.baseUrl, + options.kernelConnection.serverId, + ex + ); + } + } + + Cancellation.throwIfCanceled(options.token); + // Figure out the working directory we need for our new notebook. This is only necessary for local. + const workingDirectory = isLocalConnection(options.kernelConnection) + ? await this.workspaceService.computeWorkingDirectory(options.resource) + : ''; + Cancellation.throwIfCanceled(options.token); + // Start a session (or use the existing one if allowed) + const session = await options.sessionManager.startNew( + options.resource, + options.kernelConnection, + Uri.file(workingDirectory), + options.ui, + options.token, + options.creator + ); + if (options.token.isCancellationRequested) { + // Even if this is a remote kernel, we should shut this down as it's not needed. + session.shutdown().catch(noop); + } + Cancellation.throwIfCanceled(options.token); + traceInfo(`Started session for kernel ${options.kernelConnection.kind}:${options.kernelConnection.id}`); + return session; + } +} diff --git a/src/kernels/jupyter/session/jupyterSessionManager.ts b/src/kernels/jupyter/session/jupyterSessionManager.ts index 420e0ebe436..786c33e8815 100644 --- a/src/kernels/jupyter/session/jupyterSessionManager.ts +++ b/src/kernels/jupyter/session/jupyterSessionManager.ts @@ -60,6 +60,9 @@ export class JupyterSessionManager implements IJupyterSessionManager { private _jupyterlab?: typeof import('@jupyterlab/services'); private readonly userAllowsInsecureConnections: IPersistentState; private disposed?: boolean; + public get isDisposed() { + return this.disposed === true; + } private get jupyterlab(): typeof import('@jupyterlab/services') { if (!this._jupyterlab) { // eslint-disable-next-line @typescript-eslint/no-require-imports diff --git a/src/kernels/jupyter/session/jupyterSessionManagerFactory.ts b/src/kernels/jupyter/session/jupyterSessionManagerFactory.ts index a911bbba63e..a9236418cd1 100644 --- a/src/kernels/jupyter/session/jupyterSessionManagerFactory.ts +++ b/src/kernels/jupyter/session/jupyterSessionManagerFactory.ts @@ -5,7 +5,12 @@ import { inject, injectable, named, optional } from 'inversify'; import { JupyterSessionManager } from './jupyterSessionManager'; import { IApplicationShell } from '../../../platform/common/application/types'; import { JUPYTER_OUTPUT_CHANNEL } from '../../../platform/common/constants'; -import { IConfigurationService, IOutputChannel, IPersistentStateFactory } from '../../../platform/common/types'; +import { + IAsyncDisposableRegistry, + IConfigurationService, + IOutputChannel, + IPersistentStateFactory +} from '../../../platform/common/types'; import { IJupyterConnection } from '../../types'; import { IJupyterSessionManagerFactory, @@ -30,7 +35,8 @@ export class JupyterSessionManagerFactory implements IJupyterSessionManagerFacto @inject(IJupyterRequestAgentCreator) @optional() private readonly requestAgentCreator: IJupyterRequestAgentCreator | undefined, - @inject(IJupyterRequestCreator) private readonly requestCreator: IJupyterRequestCreator + @inject(IJupyterRequestCreator) private readonly requestCreator: IJupyterRequestCreator, + @inject(IAsyncDisposableRegistry) private readonly asyncDisposables: IAsyncDisposableRegistry ) {} /** @@ -52,6 +58,7 @@ export class JupyterSessionManagerFactory implements IJupyterSessionManagerFacto this.requestAgentCreator, this.requestCreator ); + this.asyncDisposables.push(result); await result.initialize(connInfo); return result; } diff --git a/src/kernels/jupyter/types.ts b/src/kernels/jupyter/types.ts index a90bb1b3266..054011e770f 100644 --- a/src/kernels/jupyter/types.ts +++ b/src/kernels/jupyter/types.ts @@ -15,15 +15,12 @@ import { PythonEnvironment } from '../../platform/pythonEnvironments/info'; import { KernelConnectionMetadata, IJupyterConnection, - ConnectNotebookProviderOptions, - NotebookCreationOptions, IJupyterKernelConnectionSession, IJupyterKernelSpec, GetServerOptions, IKernelSocket, KernelActionSource, LiveRemoteKernelConnectionMetadata, - IKernelConnectionSession, RemoteKernelConnectionMetadata } from '../types'; import { ClassType } from '../../platform/ioc/types'; @@ -47,56 +44,12 @@ export enum JupyterInterpreterDependencyResponse { cancel } -// Talks to a jupyter ipython kernel to retrieve data for cells -export const INotebookServer = Symbol('INotebookServer'); -export interface INotebookServer extends IAsyncDisposable { - createNotebook( - resource: Resource, - kernelConnection: KernelConnectionMetadata, - cancelToken: CancellationToken, - ui: IDisplayOptions, - creator: KernelActionSource - ): Promise; - readonly connection: IJupyterConnection; -} - -export const INotebookServerFactory = Symbol('INotebookServerFactory'); -export interface INotebookServerFactory { - createNotebookServer(connection: IJupyterConnection): Promise; -} - -// Provides notebooks that talk to jupyter servers -export const IJupyterNotebookProvider = Symbol('IJupyterNotebookProvider'); -export interface IJupyterNotebookProvider { - connect(options: ConnectNotebookProviderOptions): Promise; - createNotebook(options: NotebookCreationOptions): Promise; -} - -type INotebookServerLocalOptions = { - resource: Resource; - ui: IDisplayOptions; - /** - * Whether we're only interested in local Jupyter Servers. - */ - localJupyter: true; -}; -type INotebookServerRemoteOptions = { - serverId: string; - resource: Resource; - ui: IDisplayOptions; - /** - * Whether we're only interested in local Jupyter Servers. - */ - localJupyter: false; -}; -export type INotebookServerOptions = INotebookServerLocalOptions | INotebookServerRemoteOptions; - export const IJupyterExecution = Symbol('IJupyterExecution'); export interface IJupyterExecution extends IAsyncDisposable { isNotebookSupported(cancelToken?: CancellationToken): Promise; - connectToNotebookServer(options: INotebookServerOptions, cancelToken?: CancellationToken): Promise; + connectToNotebookServer(resource: Resource, cancelToken?: CancellationToken): Promise; getUsableJupyterPython(cancelToken?: CancellationToken): Promise; - getServer(options: INotebookServerOptions): Promise; + getServer(resource: Resource): Promise; getNotebookError(): Promise; refreshCommands(): Promise; } @@ -118,6 +71,7 @@ export interface IJupyterSessionManagerFactory { } export interface IJupyterSessionManager extends IAsyncDisposable { + readonly isDisposed: boolean; startNew( resource: Resource, kernelConnection: KernelConnectionMetadata, @@ -187,11 +141,15 @@ export interface INbConvertExportToPythonService { } export const IJupyterServerProvider = Symbol('IJupyterServerProvider'); +/** + * Provides a wrapper around a local Jupyter Notebook Server. + */ export interface IJupyterServerProvider { /** - * Gets the server used for starting notebooks + * Stats the local Jupyter Notebook server (if not already started) + * and returns the connection information. */ - getOrCreateServer(options: GetServerOptions): Promise; + getOrCreateServer(options: GetServerOptions): Promise; } export interface IJupyterServerUri { diff --git a/src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts b/src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts index 3a61ee8161b..16a0046618d 100644 --- a/src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts +++ b/src/kernels/raw/finder/contributedLocalKernelSpecFinder.node.ts @@ -14,7 +14,7 @@ import { DataScience } from '../../../platform/common/utils/localize'; import { IPythonExtensionChecker } from '../../../platform/api/types'; import { IInterpreterService } from '../../../platform/interpreter/contracts'; import { ContributedKernelFinderKind, IContributedKernelFinder } from '../../internalTypes'; -import { PYTHON_LANGUAGE } from '../../../platform/common/constants'; +import { PYTHON_LANGUAGE, isUnitTestExecution } from '../../../platform/common/constants'; import { PromiseMonitor } from '../../../platform/common/utils/promises'; import { getKernelRegistrationInfo } from '../../helpers'; import { createDeferred, Deferred } from '../../../platform/common/utils/async'; @@ -238,19 +238,30 @@ export class ContributedLocalKernelSpecFinder clearTimeout(this.cacheLoggingTimeout); } // Reduce the logging, as this can get written a lot, - this.cacheLoggingTimeout = setTimeout(() => { - traceVerbose( - `Updating cache with Local kernels ${values - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Added = ${added - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Updated = ${updated - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Removed = ${removed - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}` - ); - }, 15_000); + this.cacheLoggingTimeout = setTimeout( + () => { + traceVerbose( + `Updating cache with Local kernels ${values + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Added = ${added + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Updated = ${updated + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Removed = ${removed + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}` + ); + }, + isUnitTestExecution() ? 0 : 15_000 + ); this.disposables.push(new Disposable(() => clearTimeout(this.cacheLoggingTimeout))); } } catch (ex) { diff --git a/src/kernels/raw/finder/contributedLocalPythonEnvFinder.node.ts b/src/kernels/raw/finder/contributedLocalPythonEnvFinder.node.ts index 826f222c5b4..4894e5fffba 100644 --- a/src/kernels/raw/finder/contributedLocalPythonEnvFinder.node.ts +++ b/src/kernels/raw/finder/contributedLocalPythonEnvFinder.node.ts @@ -19,6 +19,7 @@ import { getKernelRegistrationInfo } from '../../helpers'; import { ILocalKernelFinder } from './localKernelSpecFinderBase.node'; import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node'; import { getDisplayPath } from '../../../platform/common/platform/fs-paths'; +import { isUnitTestExecution } from '../../../platform/common/constants'; // This class searches for local kernels. // First it searches on a global persistent state, then on the installed python interpreters, @@ -176,19 +177,30 @@ export class ContributedLocalPythonEnvFinder clearTimeout(this.cacheLoggingTimeout); } // Reduce the logging, as this can get written a lot, - this.cacheLoggingTimeout = setTimeout(() => { - traceVerbose( - `Updating cache with Python kernels ${values - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Added = ${added - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Updated = ${updated - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}, Removed = ${removed - .map((k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'`) - .join(', ')}` - ); - }, 15_000); + this.cacheLoggingTimeout = setTimeout( + () => { + traceVerbose( + `Updating cache with Python kernels ${values + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Added = ${added + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Updated = ${updated + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}, Removed = ${removed + .map( + (k) => `${k.kind}:'${k.id} (interpreter id = ${getDisplayPath(k.interpreter?.id)})'` + ) + .join(', ')}` + ); + }, + isUnitTestExecution() ? 0 : 15_000 + ); this.disposables.push(new Disposable(() => clearTimeout(this.cacheLoggingTimeout))); } } catch (ex) { diff --git a/src/kernels/types.ts b/src/kernels/types.ts index a2c726dec4a..ba155b86737 100644 --- a/src/kernels/types.ts +++ b/src/kernels/types.ts @@ -551,6 +551,7 @@ export interface IBaseKernelConnectionSession extends IAsyncDisposable { isServerSession(): this is IJupyterKernelConnectionSession; onSessionStatusChanged: Event; onDidDispose: Event; + onDidShutdown: Event; interrupt(): Promise; restart(): Promise; waitForIdle(timeout: number, token: CancellationToken): Promise; @@ -675,28 +676,18 @@ export interface IJupyterKernelSpec { | 'registeredByNewVersionOfExtForCustomKernelSpec'; } -export type GetServerOptions = - | { - ui: IDisplayOptions; - localJupyter: true; - token: CancellationToken | undefined; - resource: Resource; - serverId?: undefined; - } - | { - ui: IDisplayOptions; - localJupyter: false; - token: CancellationToken | undefined; - resource: Resource; - serverId: string; - }; +export type GetServerOptions = { + ui: IDisplayOptions; + token: CancellationToken | undefined; + resource: Resource; +}; // Options for connecting to a notebook provider export type ConnectNotebookProviderOptions = GetServerOptions; /** * Options for getting a notebook */ -export type NotebookCreationOptions = { +export type KernelConnectionSessionCreationOptions = { resource: Resource; ui: IDisplayOptions; kernelConnection: KernelConnectionMetadata; @@ -725,7 +716,7 @@ export interface IKernelConnectionSessionCreator { /** * Creates a notebook. */ - create(options: NotebookCreationOptions): Promise; + create(options: KernelConnectionSessionCreationOptions): Promise; } export interface IKernelSocket { diff --git a/src/test/datascience/mockJupyterServer.ts b/src/test/datascience/mockJupyterServer.ts deleted file mode 100644 index 89a98454bc1..00000000000 --- a/src/test/datascience/mockJupyterServer.ts +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -import { Uri } from 'vscode'; -import { INotebookServer } from '../../kernels/jupyter/types'; -import { IJupyterConnection, IJupyterKernelConnectionSession } from '../../kernels/types'; -import { TemporaryFile } from '../../platform/common/platform/types'; - -export class MockJupyterServer implements INotebookServer { - constructor(public connection: IJupyterConnection) {} - private notebookFile: TemporaryFile | undefined; - - public async createNotebook(_resource: Uri): Promise { - throw new Error('Not implemented'); - } - public async dispose(): Promise { - if (this.connection) { - this.connection.dispose(); // This should kill the process that's running - } - if (this.notebookFile) { - this.notebookFile.dispose(); // This destroy any unwanted kernel specs if necessary - this.notebookFile = undefined; - } - } -} diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index e4642632364..50dd711fb14 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -46,7 +46,6 @@ import { workspace } from 'vscode'; import { DebugProtocol } from 'vscode-debugprotocol'; -import { DisplayOptions } from '../../../kernels/displayOptions'; import { CellOutputMimeTypes, NotebookCellStateTracker, @@ -62,7 +61,6 @@ import { import { IKernelFinder, IKernelProvider, - IJupyterServerConnector, IThirdPartyKernelProvider, PythonKernelConnectionMetadata, RemoteKernelSpecConnectionMetadata @@ -104,6 +102,7 @@ import { noop } from '../../core'; import { closeActiveWindows, isInsiders } from '../../initialize'; import { verifySelectedControllerIsRemoteForRemoteTests } from '../helpers'; import { ControllerPreferredService } from './controllerPreferredService'; +import { JupyterConnection } from '../../../kernels/jupyter/connection/jupyterConnection'; // Running in Conda environments, things can be a little slower. export const defaultNotebookTestTimeout = 60_000; @@ -343,7 +342,7 @@ export async function ensureNewNotebooksHavePythonCells() { async function shutdownRemoteKernels() { const api = await initialize(); const serverUriStorage = api.serviceContainer.get(IJupyterServerUriStorage); - const jupyterServerConnector = api.serviceContainer.get(IJupyterServerConnector); + const jupyterConnection = api.serviceContainer.get(JupyterConnection); const jupyterSessionManagerFactory = api.serviceContainer.get(IJupyterSessionManagerFactory); const uri = await serverUriStorage.getRemoteUri(); @@ -353,11 +352,7 @@ async function shutdownRemoteKernels() { const cancelToken = new CancellationTokenSource(); let sessionManager: IJupyterSessionManager | undefined; try { - const connection = await jupyterServerConnector.connect({ - resource: undefined, - ui: new DisplayOptions(true), - localJupyter: false, - token: cancelToken.token, + const connection = await jupyterConnection.createConnectionInfo({ serverId: serverUriStorage.currentServerId! }); if (connection.type !== 'jupyter') {