Use different method for checking if kernelspec is available (#10114)

* Use different method for checking if kernelspec is available

* Fix unit tests

Author: rchiodo

pythonFiles/datascience/getJupyterKernelspecVersion.py

@@ -0,0 +1,7 @@
+# Check whether kernelspec module exists.
+import sys
+import jupyter_client
+import jupyter_client.kernelspec
+
+sys.stdout.write(jupyter_client.__version__)
+sys.stdout.flush()

src/client/datascience/jupyter/interpreter/jupyterCommand.ts

@@ -201,31 +201,49 @@ export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterComma
             return output;
         }
 
-        // We're only interested in `python -m jupyter kernelspec list --json`
-        const interpreter = await this.interpreter();
-        if (
-            !interpreter ||
-            this.moduleName.toLowerCase() !== 'jupyter' ||
-            this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() ||
-            args.join(' ').toLowerCase() !== 'list --json'
-        ) {
+        const defaultAction = () => {
             if (exception) {
                 throw exception;
             }
             return output;
+        };
+
+        // We're only interested in `python -m jupyter kernelspec`
+        const interpreter = await this.interpreter();
+        if (!interpreter || this.moduleName.toLowerCase() !== 'jupyter' || this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase()) {
+            return defaultAction();
         }
+
+        // Otherwise try running a script instead.
         try {
-            // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
-            const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({ interpreter, bypassCondaExecution: true });
-            return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')], { ...options, throwOnStdErr: true });
+            if (args.join(' ').toLowerCase() === 'list --json') {
+                // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
+                return this.getKernelSpecList(interpreter, options);
+            } else if (args.join(' ').toLowerCase() === '--version') {
+                // Try getting kernelspec version using python script, if that fails (even if there's output in stderr) rethrow original exception.
+                return this.getKernelSpecVersion(interpreter, options);
+            }
         } catch (innerEx) {
             traceError('Failed to get a list of the kernelspec using python script', innerEx);
-            // Rethrow original exception.
-            if (exception) {
-                throw exception;
-            }
-            return output;
         }
+        return defaultAction();
+    }
+
+    private async getKernelSpecList(interpreter: PythonInterpreter, options: SpawnOptions) {
+        // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
+        const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({
+            interpreter,
+            bypassCondaExecution: true
+        });
+        return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')], { ...options, throwOnStdErr: true });
+    }
+    private async getKernelSpecVersion(interpreter: PythonInterpreter, options: SpawnOptions) {
+        // Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
+        const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({
+            interpreter,
+            bypassCondaExecution: true
+        });
+        return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernelspecVersion.py')], { ...options, throwOnStdErr: true });
     }
 }
 

src/client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService.ts

@@ -8,13 +8,13 @@ import { CancellationToken } from 'vscode';
 import { IApplicationShell } from '../../../common/application/types';
 import { Cancellation, createPromiseFromCancellation, wrapCancellationTokens } from '../../../common/cancellation';
 import { ProductNames } from '../../../common/installer/productNames';
-import { IPythonExecutionFactory } from '../../../common/process/types';
 import { IInstaller, InstallerResponse, Product } from '../../../common/types';
 import { Common, DataScience } from '../../../common/utils/localize';
 import { noop } from '../../../common/utils/misc';
 import { PythonInterpreter } from '../../../interpreter/contracts';
 import { sendTelemetryEvent } from '../../../telemetry';
-import { HelpLinks, Telemetry } from '../../constants';
+import { HelpLinks, JupyterCommands, Telemetry } from '../../constants';
+import { IJupyterCommandFactory } from '../../types';
 import { JupyterInstallError } from '../jupyterInstallError';
 
 export enum JupyterInterpreterDependencyResponse {
@@ -108,7 +108,7 @@ export class JupyterInterpreterDependencyService {
     constructor(
         @inject(IApplicationShell) private readonly applicationShell: IApplicationShell,
         @inject(IInstaller) private readonly installer: IInstaller,
-        @inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory
+        @inject(IJupyterCommandFactory) private readonly commandFactory: IJupyterCommandFactory
     ) {}
     /**
      * Configures the python interpreter to ensure it can run Jupyter server by installing any missing dependencies.
@@ -270,13 +270,10 @@ export class JupyterInterpreterDependencyService {
      * @returns {Promise<boolean>}
      * @memberof JupyterInterpreterConfigurationService
      */
-    private async isKernelSpecAvailable(interpreter: PythonInterpreter, token?: CancellationToken): Promise<boolean> {
-        const execService = await this.pythonExecFactory.createActivatedEnvironment({ interpreter, allowEnvironmentFetchExceptions: true, bypassCondaExecution: true });
-        if (Cancellation.isCanceled(token)) {
-            return false;
-        }
-        return execService
-            .execModule('jupyter', ['kernelspec', '--version'], { throwOnStdErr: true })
+    private async isKernelSpecAvailable(interpreter: PythonInterpreter, _token?: CancellationToken): Promise<boolean> {
+        const command = this.commandFactory.createInterpreterCommand(JupyterCommands.KernelSpecCommand, 'jupyter', ['-m', 'jupyter', 'kernelspec'], interpreter, false);
+        return command
+            .exec(['--version'], { throwOnStdErr: true })
             .then(() => true)
             .catch(() => {
                 sendTelemetryEvent(Telemetry.KernelSpecNotFound);
@@ -298,9 +295,10 @@ export class JupyterInterpreterDependencyService {
      */
     private async checkKernelSpecAvailability(interpreter: PythonInterpreter, token?: CancellationToken): Promise<JupyterInterpreterDependencyResponse> {
         if (await this.isKernelSpecAvailable(interpreter)) {
-            sendTelemetryEvent(Telemetry.JupyterInstalledButNotKernelSpecModule);
             return JupyterInterpreterDependencyResponse.ok;
         }
+        // Indicate no kernel spec module.
+        sendTelemetryEvent(Telemetry.JupyterInstalledButNotKernelSpecModule);
         if (Cancellation.isCanceled(token)) {
             return JupyterInterpreterDependencyResponse.cancel;
         }

src/test/datascience/jupyter/interpreter/jupyterInterpreterDependencyService.unit.test.ts

@@ -4,25 +4,26 @@
 'use strict';
 
 import { assert } from 'chai';
-import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito';
+import { anything, instance, mock, verify, when } from 'ts-mockito';
 import { ApplicationShell } from '../../../../client/common/application/applicationShell';
 import { IApplicationShell } from '../../../../client/common/application/types';
 import { ProductInstaller } from '../../../../client/common/installer/productInstaller';
-import { PythonExecutionFactory } from '../../../../client/common/process/pythonExecutionFactory';
-import { PythonExecutionService } from '../../../../client/common/process/pythonProcess';
-import { IPythonExecutionService } from '../../../../client/common/process/types';
 import { IInstaller, InstallerResponse, Product } from '../../../../client/common/types';
 import { DataScience } from '../../../../client/common/utils/localize';
 import { Architecture } from '../../../../client/common/utils/platform';
+import { InterpreterJupyterKernelSpecCommand, JupyterCommandFactory } from '../../../../client/datascience/jupyter/interpreter/jupyterCommand';
 import { JupyterInterpreterDependencyResponse, JupyterInterpreterDependencyService } from '../../../../client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService';
+import { IJupyterCommand, IJupyterCommandFactory } from '../../../../client/datascience/types';
 import { InterpreterType, PythonInterpreter } from '../../../../client/interpreter/contracts';
 
-// tslint:disable-next-line: max-func-body-length
+// tslint:disable: max-func-body-length no-any
+
 suite('Data Science - Jupyter Interpreter Configuration', () => {
     let configuration: JupyterInterpreterDependencyService;
     let appShell: IApplicationShell;
     let installer: IInstaller;
-    let pythonExecService: IPythonExecutionService;
+    let commandFactory: IJupyterCommandFactory;
+    let command: IJupyterCommand;
     const pythonInterpreter: PythonInterpreter = {
         path: '',
         architecture: Architecture.Unknown,
@@ -33,14 +34,14 @@ suite('Data Science - Jupyter Interpreter Configuration', () => {
     setup(() => {
         appShell = mock(ApplicationShell);
         installer = mock(ProductInstaller);
-        pythonExecService = mock(PythonExecutionService);
-        const pythonExecFactory = mock(PythonExecutionFactory);
-        when(pythonExecFactory.createActivatedEnvironment(anything())).thenResolve(instance(pythonExecService));
-        // tslint:disable-next-line: no-any
-        instance(pythonExecService as any).then = undefined;
-        when(pythonExecService.execModule('jupyter', deepEqual(['kernelspec', '--version']), anything())).thenResolve({ stdout: '' });
+        commandFactory = mock(JupyterCommandFactory);
+        command = mock(InterpreterJupyterKernelSpecCommand);
+        instance(commandFactory as any).then = undefined;
+        instance(command as any).then = undefined;
+        when(commandFactory.createInterpreterCommand(anything(), anything(), anything(), anything(), anything())).thenReturn(instance(command));
+        when(command.exec(anything(), anything())).thenResolve({ stdout: '' });
 
-        configuration = new JupyterInterpreterDependencyService(instance(appShell), instance(installer), instance(pythonExecFactory));
+        configuration = new JupyterInterpreterDependencyService(instance(appShell), instance(installer), instance(commandFactory));
     });
     test('Return ok if all dependencies are installed', async () => {
         when(installer.isInstalled(Product.jupyter, pythonInterpreter)).thenResolve(true);
@@ -72,7 +73,7 @@ suite('Data Science - Jupyter Interpreter Configuration', () => {
             // tslint:disable-next-line: no-any
             DataScience.jupyterInstall() as any
         );
-        when(pythonExecService.execModule('jupyter', deepEqual(['kernelspec', '--version']), anything())).thenReject(new Error('Not found'));
+        when(command.exec(anything(), anything())).thenReject(new Error('Not found'));
         when(installer.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed);
 
         const response = await configuration.installMissingDependencies(pythonInterpreter);