Do not Pre-fetch kernels upon activation for remote jupyter connectio…

…ns (#5878)

* News entry

* misc

news/3 Code Health/5846.md

@@ -0,0 +1 @@
+When using remote Jupyter connections pre-fetch kernels only when opening a notebook.

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<VSCodeNotebookController[]> | undefined;
+    private controllersPromise?: Promise<VSCodeNotebookController[]>;
 
-    private isLocalLaunch: boolean;
     private cancelToken: CancellationTokenSource | undefined;
-    private _allowRemoteConnection = createDeferred<void>();
+    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<void> {
-        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<VSCodeNotebookController[] | undefined> {
+    public async getNotebookControllers(): Promise<VSCodeNotebookController[]> {
+        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,

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<NotebookControllerManager>(INotebookControllerManager);
-        notebookControllerManager.allowRemoteConnection.resolve();
     } else {
         traceInfo(`Jupyter not started and set to local`); // This is the default
     }