Merged PR posit-dev/positron-python#117: prompt to install ipykernel on runtime start instead of registration

Merge pull request #117 from posit-dev/install-ipykernel-on-start

prompt to install ipykernel on runtime start instead of registration
--------------------
Commit message for posit-dev/positron-python@0c1d9c0:

prompt to install ipykernel on runtime start instead of registration

This required moving that functionality to the language runtime.

It also lets us throw an error if the install failed, which currently gets
rendered in the console. We can replace that with a pop-up in future.

Addresses #436.


Authored-by: Wasim Lorgat <[email protected]>
Signed-off-by: Wasim Lorgat <[email protected]>

Author: seeM

extensions/positron-python/src/client/activation/jedi/positronLanguageRuntimes.ts

@@ -14,8 +14,7 @@ import { Disposable, DocumentFilter, LanguageClientOptions } from 'vscode-langua
 
 import * as semver from 'semver';
 import { EXTENSION_ROOT_DIR, PYTHON_LANGUAGE } from '../../common/constants';
-import { IConfigurationService, IDisposableRegistry, IInstaller, InstallerResponse, Product, Resource } from '../../common/types';
-import { InstallOptions } from '../../common/installer/types';
+import { IConfigurationService, IDisposableRegistry, IInstaller, Product, Resource } from '../../common/types';
 import { IInterpreterService } from '../../interpreter/contracts';
 import { IServiceContainer } from '../../ioc/types';
 import { traceVerbose } from '../../logging';
@@ -43,10 +42,6 @@ export class PositronJediLanguageServerProxy implements ILanguageServerProxy {
 
     private readonly installer: IInstaller;
 
-    // Using a process to install modules avoids using the terminal service,
-    // which has issues waiting for the outcome of the install.
-    private readonly installOptions: InstallOptions = { installAsProcess: true };
-
     private registered = false;
 
     private readonly minimumSupportedVersion = '3.8.0';
@@ -65,7 +60,7 @@ export class PositronJediLanguageServerProxy implements ILanguageServerProxy {
             traceVerbose("Unable to read package.json to determine our extension version", e);
         }
 
-        this.disposables = serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
+        this.disposables = this.serviceContainer.get<IDisposableRegistry>(IDisposableRegistry);
         this.installer = this.serviceContainer.get<IInstaller>(IInstaller);
     }
 
@@ -99,16 +94,6 @@ export class PositronJediLanguageServerProxy implements ILanguageServerProxy {
         // Extend LSP support to include unsaved editors
         options.documentSelector = this.initDocumentSelector(options.documentSelector as DocumentFilter[]);
 
-        // Offer to install the ipykernel module for the preferred interpreter, if it is missing
-        const hasKernel = await this.installer.isInstalled(Product.ipykernel, interpreter);
-        if (!hasKernel) {
-            const response = await this.installer.promptToInstall(Product.ipykernel,
-                interpreter, undefined, undefined, this.installOptions);
-            if (response === InstallerResponse.Installed) {
-                traceVerbose(`Successfully installed ipykernel for ${interpreter?.displayName}`);
-            }
-        }
-
         // Register available python interpreters as language runtimes with our Jupyter Adapter
         this.withActiveExtention(ext, async () => {
             // Create typesafe reference to the Jupyter Adapter's API
@@ -245,7 +230,7 @@ export class PositronJediLanguageServerProxy implements ILanguageServerProxy {
 
         // Create an adapter for the kernel as our language runtime
         const runtime = new PythonLanguageRuntime(
-            kernelSpec, metadata, jupyterAdapterApi, options);
+            kernelSpec, metadata, jupyterAdapterApi, options, interpreter, this.installer);
 
         // Register our language runtime provider
         return positron.runtime.registerLanguageRuntime(runtime);

extensions/positron-python/src/client/activation/jedi/pythonLanguageRuntime.ts

@@ -10,8 +10,12 @@ import * as positron from 'positron';
 import * as vscode from 'vscode';
 import { Disposable, LanguageClient, LanguageClientOptions, ServerOptions, StreamInfo } from 'vscode-languageclient/node';
 import { PYTHON_LANGUAGE } from '../../common/constants';
+import { InstallOptions } from '../../common/installer/types';
+import { IInstaller, Product, InstallerResponse } from '../../common/types';
 import { JupyterAdapterApi, JupyterKernelSpec, JupyterLanguageRuntime } from '../../jupyter-adapter.d';
+import { traceVerbose } from '../../logging';
 import { ProgressReporting } from '../progress';
+import { PythonEnvironment } from '../../pythonEnvironments/info';
 
 /**
  * A Positron Python language runtime that wraps a Jupyter kernel runtime.
@@ -26,6 +30,10 @@ export class PythonLanguageRuntime implements JupyterLanguageRuntime, Disposable
 
     private readonly disposables: Disposable[] = [];
 
+    // Using a process to install modules avoids using the terminal service,
+    // which has issues waiting for the outcome of the install.
+    private readonly installOptions: InstallOptions = { installAsProcess: true };
+
     /**
      * Create a new PythonLanguageRuntime object to wrap a Jupyter kernel.
      *
@@ -37,7 +45,10 @@ export class PythonLanguageRuntime implements JupyterLanguageRuntime, Disposable
         readonly kernelSpec: JupyterKernelSpec,
         readonly metadata: positron.LanguageRuntimeMetadata,
         readonly adapterApi: JupyterAdapterApi,
-        readonly languageClientOptions: LanguageClientOptions) {
+        readonly languageClientOptions: LanguageClientOptions,
+        private readonly interpreter: PythonEnvironment | undefined,
+        private readonly installer: IInstaller,
+    ) {
 
         this._kernel = adapterApi.adaptKernel(kernelSpec, metadata);
 
@@ -91,10 +102,31 @@ export class PythonLanguageRuntime implements JupyterLanguageRuntime, Disposable
         this._kernel.replyToPrompt(id, reply);
     }
 
-    start(): Thenable<positron.LanguageRuntimeInfo> {
+    async start(): Promise<positron.LanguageRuntimeInfo> {
+        await this._installIpykernel();
         return this._kernel.start();
     }
 
+    private async _installIpykernel(): Promise<void> {
+        // Offer to install the ipykernel module for the preferred interpreter, if it is missing.
+        // Thow an error if it could not be installed.
+        const hasKernel = await this.installer.isInstalled(Product.ipykernel, this.interpreter);
+        if (!hasKernel) {
+            const response = await this.installer.promptToInstall(Product.ipykernel,
+                this.interpreter, undefined, undefined, this.installOptions);
+            switch (response) {
+                case InstallerResponse.Installed:
+                    traceVerbose(`Successfully installed ipykernel for ${this.interpreter?.displayName}`);
+                    break;
+                case InstallerResponse.Ignore:
+                case InstallerResponse.Disabled:
+                    throw new Error(`Could not start runtime: failed to install ipykernel for ${this.interpreter?.displayName}.`);
+                default:
+                    throw new Error(`Unknown installer response type: ${response}`);
+            }
+        }
+    }
+
     async interrupt(): Promise<void> {
         return this._kernel.interrupt();
     }