diff --git a/news/3 Code Health/5846.md b/news/3 Code Health/5846.md new file mode 100644 index 00000000000..af45ad5a62c --- /dev/null +++ b/news/3 Code Health/5846.md @@ -0,0 +1 @@ +When using remote Jupyter connections pre-fetch kernels only when opening a notebook. diff --git a/src/client/datascience/notebook/notebookControllerManager.ts b/src/client/datascience/notebook/notebookControllerManager.ts index 2b1e98fccd1..c0019420b48 100644 --- a/src/client/datascience/notebook/notebookControllerManager.ts +++ b/src/client/datascience/notebook/notebookControllerManager.ts @@ -39,8 +39,6 @@ import { NotebookIPyWidgetCoordinator } from '../ipywidgets/notebookIPyWidgetCoo import { IPyWidgetMessages } from '../interactive-common/interactiveWindowTypes'; import { InterpreterPackages } from '../telemetry/interpreterPackages'; import { sendTelemetryEvent } from '../../telemetry'; -import { createDeferred, Deferred } from '../../common/utils/async'; -import { IS_REMOTE_NATIVE_TEST } from '../../../test/constants'; /** * This class tracks notebook documents that are open and the provides NotebookControllers for * each of them @@ -59,11 +57,10 @@ export class NotebookControllerManager implements INotebookControllerManager, IE }>; // Promise to resolve when we have loaded our controllers - private controllersPromise: Promise | undefined; + private controllersPromise?: Promise; - private isLocalLaunch: boolean; private cancelToken: CancellationTokenSource | undefined; - private _allowRemoteConnection = createDeferred(); + private readonly isLocalLaunch: boolean; constructor( @inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook, @inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry, @@ -88,22 +85,12 @@ export class NotebookControllerManager implements INotebookControllerManager, IE }>(); this.disposables.push(this._onNotebookControllerSelected); this.isLocalLaunch = isLocalLaunch(this.configuration); - - // For remote tests, we need to wait until we start jupyter and assign the server URI - // so we wait on this connection, for non remote tests just run - if (!IS_REMOTE_NATIVE_TEST) { - this._allowRemoteConnection.resolve(); - } } get onNotebookControllerSelected() { return this._onNotebookControllerSelected.event; } - get allowRemoteConnection(): Deferred { - return this._allowRemoteConnection; - } - public activate() { // Sign up for document either opening or closing this.notebook.onDidOpenNotebookDocument(this.onDidOpenNotebookDocument, this, this.disposables); @@ -112,10 +99,10 @@ export class NotebookControllerManager implements INotebookControllerManager, IE // Be aware of if we need to re-look for kernels on extension change this.extensions.onDidChange(this.onDidChangeExtensions, this, this.disposables); - this.controllersPromise = this.loadNotebookControllers().catch((error) => { - traceError('Error loading notebook controllers', error); - throw error; - }); + if (this.isLocalLaunch) { + // Pre-warm fetching local kernels, for remote connections fetch as and when needed. + this.getNotebookControllers().catch(traceError); + } } // Look up what NotebookController is currently selected for the given notebook document @@ -126,7 +113,13 @@ export class NotebookControllerManager implements INotebookControllerManager, IE } // Find all the notebook controllers that we have registered - public async getNotebookControllers(): Promise { + public async getNotebookControllers(): Promise { + if (!this.controllersPromise) { + this.controllersPromise = this.loadNotebookControllers().catch((error) => { + traceError('Error loading notebook controllers', error); + throw error; + }); + } return this.controllersPromise; } @@ -206,14 +199,9 @@ export class NotebookControllerManager implements INotebookControllerManager, IE // For the given document, find the notebook controller that matches this kernel connection and associate the two private async setPreferredController(document: NotebookDocument, kernelConnection: KernelConnectionMetadata) { - if (!this.controllersPromise) { - // Should not happen as this promise is assigned in activate - return; - } - // Wait for our controllers to be loaded before we try to set a preferred on // can happen if a document is opened quick and we have not yet loaded our controllers - const controllers = await this.controllersPromise; + const controllers = await this.getNotebookControllers(); const targetController = controllers.find((value) => { // Check for a connection match @@ -364,12 +352,6 @@ export class NotebookControllerManager implements INotebookControllerManager, IE if (this.isLocalLaunch) { kernels = await this.localKernelFinder.listKernels(resource, token); } else { - // In remote CI test we need to wait until we start up our server and input our URI before we - // try to connect to the server to get kernel connection info - if (IS_REMOTE_NATIVE_TEST) { - await this.allowRemoteConnection.promise; - } - const connection = await this.notebookProvider.connect({ getOnly: false, resource: resource, diff --git a/src/test/datascience/notebook/helper.ts b/src/test/datascience/notebook/helper.ts index 8395627a4db..a2152b4ae6d 100644 --- a/src/test/datascience/notebook/helper.ts +++ b/src/test/datascience/notebook/helper.ts @@ -66,7 +66,6 @@ import { JupyterServer } from '../jupyterServer'; import { NotebookEditorProvider } from '../../../client/datascience/notebook/notebookEditorProvider'; import { VSCodeNotebookProvider } from '../../../client/datascience/constants'; import { VSCodeNotebookController } from '../../../client/datascience/notebook/vscodeNotebookController'; -import { NotebookControllerManager } from '../../../client/datascience/notebook/notebookControllerManager'; // Running in Conda environments, things can be a little slower. const defaultTimeout = IS_CONDA_TEST ? 30_000 : 15_000; @@ -406,11 +405,6 @@ export async function startJupyterServer(api?: IExtensionTestApi) { const uriString = decodeURIComponent(uri.toString()); traceInfo(`Jupyter started and listening at ${uriString}`); await selector.setJupyterURIToRemote(uriString); - - // Once we have set the URI allow kernel loading to continue, we don't want this to happen ealier - // as it will pop up a server selector if the URI is not set yet - const notebookControllerManager = serviceContainer.get(INotebookControllerManager); - notebookControllerManager.allowRemoteConnection.resolve(); } else { traceInfo(`Jupyter not started and set to local`); // This is the default }