Skip to content

Commit 6c0f326

Browse files
rchiodorchiodo
authored
Use different method for checking if kernelspec is available (microsoft#10114)
* Use different method for checking if kernelspec is available * Fix unit tests
1 parent 077c42a commit 6c0f326

4 files changed

Lines changed: 83 additions & 53 deletions

File tree

‎pythonFiles/datascience/getJupyterKernelspecVersion.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# Check whether kernelspec module exists.
2+
import sys
3+
import jupyter_client
4+
import jupyter_client.kernelspec
5+
6+
sys.stdout.write(jupyter_client.__version__)
7+
sys.stdout.flush()

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

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -222,37 +222,59 @@ export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterComma
222222
return output;
223223
}
224224

225-
// We're only interested in `python -m jupyter kernelspec list --json`
225+
const defaultAction = () => {
226+
if (exception) {
227+
throw exception;
228+
}
229+
return output;
230+
};
231+
232+
// We're only interested in `python -m jupyter kernelspec`
226233
const interpreter = await this.interpreter();
227234
if (
228235
!interpreter ||
229236
this.moduleName.toLowerCase() !== 'jupyter' ||
230-
this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() ||
231-
args.join(' ').toLowerCase() !== 'list --json'
237+
this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase()
232238
) {
233-
if (exception) {
234-
throw exception;
235-
}
236-
return output;
239+
return defaultAction();
237240
}
241+
242+
// Otherwise try running a script instead.
238243
try {
239-
// Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
240-
const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({
241-
interpreter,
242-
bypassCondaExecution: true
243-
});
244-
return activatedEnv.exec(
245-
[path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')],
246-
{ ...options, throwOnStdErr: true }
247-
);
244+
if (args.join(' ').toLowerCase() === 'list --json') {
245+
// Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
246+
return this.getKernelSpecList(interpreter, options);
247+
} else if (args.join(' ').toLowerCase() === '--version') {
248+
// Try getting kernelspec version using python script, if that fails (even if there's output in stderr) rethrow original exception.
249+
return this.getKernelSpecVersion(interpreter, options);
250+
}
248251
} catch (innerEx) {
249252
traceError('Failed to get a list of the kernelspec using python script', innerEx);
250-
// Rethrow original exception.
251-
if (exception) {
252-
throw exception;
253-
}
254-
return output;
255253
}
254+
return defaultAction();
255+
}
256+
257+
private async getKernelSpecList(interpreter: PythonInterpreter, options: SpawnOptions) {
258+
// Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
259+
const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({
260+
interpreter,
261+
bypassCondaExecution: true
262+
});
263+
return activatedEnv.exec(
264+
[path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')],
265+
{ ...options, throwOnStdErr: true }
266+
);
267+
}
268+
private async getKernelSpecVersion(interpreter: PythonInterpreter, options: SpawnOptions) {
269+
// Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
270+
const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({
271+
interpreter,
272+
bypassCondaExecution: true
273+
});
274+
return activatedEnv.exec(
275+
[path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernelspecVersion.py')],
276+
{ ...options, throwOnStdErr: true }
277+
);
256278
}
257279
}
258280

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ import { CancellationToken } from 'vscode';
88
import { IApplicationShell } from '../../../common/application/types';
99
import { Cancellation, createPromiseFromCancellation, wrapCancellationTokens } from '../../../common/cancellation';
1010
import { ProductNames } from '../../../common/installer/productNames';
11-
import { IPythonExecutionFactory } from '../../../common/process/types';
1211
import { IInstaller, InstallerResponse, Product } from '../../../common/types';
1312
import { Common, DataScience } from '../../../common/utils/localize';
1413
import { noop } from '../../../common/utils/misc';
1514
import { PythonInterpreter } from '../../../interpreter/contracts';
1615
import { sendTelemetryEvent } from '../../../telemetry';
17-
import { HelpLinks, Telemetry } from '../../constants';
16+
import { HelpLinks, JupyterCommands, Telemetry } from '../../constants';
17+
import { IJupyterCommandFactory } from '../../types';
1818
import { JupyterInstallError } from '../jupyterInstallError';
1919

2020
export enum JupyterInterpreterDependencyResponse {
@@ -113,7 +113,7 @@ export class JupyterInterpreterDependencyService {
113113
constructor(
114114
@inject(IApplicationShell) private readonly applicationShell: IApplicationShell,
115115
@inject(IInstaller) private readonly installer: IInstaller,
116-
@inject(IPythonExecutionFactory) private readonly pythonExecFactory: IPythonExecutionFactory
116+
@inject(IJupyterCommandFactory) private readonly commandFactory: IJupyterCommandFactory
117117
) {}
118118
/**
119119
* Configures the python interpreter to ensure it can run Jupyter server by installing any missing dependencies.
@@ -291,17 +291,16 @@ export class JupyterInterpreterDependencyService {
291291
* @returns {Promise<boolean>}
292292
* @memberof JupyterInterpreterConfigurationService
293293
*/
294-
private async isKernelSpecAvailable(interpreter: PythonInterpreter, token?: CancellationToken): Promise<boolean> {
295-
const execService = await this.pythonExecFactory.createActivatedEnvironment({
294+
private async isKernelSpecAvailable(interpreter: PythonInterpreter, _token?: CancellationToken): Promise<boolean> {
295+
const command = this.commandFactory.createInterpreterCommand(
296+
JupyterCommands.KernelSpecCommand,
297+
'jupyter',
298+
['-m', 'jupyter', 'kernelspec'],
296299
interpreter,
297-
allowEnvironmentFetchExceptions: true,
298-
bypassCondaExecution: true
299-
});
300-
if (Cancellation.isCanceled(token)) {
301-
return false;
302-
}
303-
return execService
304-
.execModule('jupyter', ['kernelspec', '--version'], { throwOnStdErr: true })
300+
false
301+
);
302+
return command
303+
.exec(['--version'], { throwOnStdErr: true })
305304
.then(() => true)
306305
.catch(() => {
307306
sendTelemetryEvent(Telemetry.KernelSpecNotFound);
@@ -326,9 +325,10 @@ export class JupyterInterpreterDependencyService {
326325
token?: CancellationToken
327326
): Promise<JupyterInterpreterDependencyResponse> {
328327
if (await this.isKernelSpecAvailable(interpreter)) {
329-
sendTelemetryEvent(Telemetry.JupyterInstalledButNotKernelSpecModule);
330328
return JupyterInterpreterDependencyResponse.ok;
331329
}
330+
// Indicate no kernel spec module.
331+
sendTelemetryEvent(Telemetry.JupyterInstalledButNotKernelSpecModule);
332332
if (Cancellation.isCanceled(token)) {
333333
return JupyterInterpreterDependencyResponse.cancel;
334334
}

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

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,32 @@
44
'use strict';
55

66
import { assert } from 'chai';
7-
import { anything, deepEqual, instance, mock, verify, when } from 'ts-mockito';
7+
import { anything, instance, mock, verify, when } from 'ts-mockito';
88
import { ApplicationShell } from '../../../../client/common/application/applicationShell';
99
import { IApplicationShell } from '../../../../client/common/application/types';
1010
import { ProductInstaller } from '../../../../client/common/installer/productInstaller';
11-
import { PythonExecutionFactory } from '../../../../client/common/process/pythonExecutionFactory';
12-
import { PythonExecutionService } from '../../../../client/common/process/pythonProcess';
13-
import { IPythonExecutionService } from '../../../../client/common/process/types';
1411
import { IInstaller, InstallerResponse, Product } from '../../../../client/common/types';
1512
import { DataScience } from '../../../../client/common/utils/localize';
1613
import { Architecture } from '../../../../client/common/utils/platform';
14+
import {
15+
InterpreterJupyterKernelSpecCommand,
16+
JupyterCommandFactory
17+
} from '../../../../client/datascience/jupyter/interpreter/jupyterCommand';
1718
import {
1819
JupyterInterpreterDependencyResponse,
1920
JupyterInterpreterDependencyService
2021
} from '../../../../client/datascience/jupyter/interpreter/jupyterInterpreterDependencyService';
22+
import { IJupyterCommand, IJupyterCommandFactory } from '../../../../client/datascience/types';
2123
import { InterpreterType, PythonInterpreter } from '../../../../client/interpreter/contracts';
2224

23-
// tslint:disable: max-func-body-length
25+
// tslint:disable: max-func-body-length no-any
2426

2527
suite('Data Science - Jupyter Interpreter Configuration', () => {
2628
let configuration: JupyterInterpreterDependencyService;
2729
let appShell: IApplicationShell;
2830
let installer: IInstaller;
29-
let pythonExecService: IPythonExecutionService;
31+
let commandFactory: IJupyterCommandFactory;
32+
let command: IJupyterCommand;
3033
const pythonInterpreter: PythonInterpreter = {
3134
path: '',
3235
architecture: Architecture.Unknown,
@@ -37,19 +40,19 @@ suite('Data Science - Jupyter Interpreter Configuration', () => {
3740
setup(() => {
3841
appShell = mock(ApplicationShell);
3942
installer = mock(ProductInstaller);
40-
pythonExecService = mock(PythonExecutionService);
41-
const pythonExecFactory = mock(PythonExecutionFactory);
42-
when(pythonExecFactory.createActivatedEnvironment(anything())).thenResolve(instance(pythonExecService));
43-
// tslint:disable-next-line: no-any
44-
instance(pythonExecService as any).then = undefined;
45-
when(pythonExecService.execModule('jupyter', deepEqual(['kernelspec', '--version']), anything())).thenResolve({
46-
stdout: ''
47-
});
43+
commandFactory = mock(JupyterCommandFactory);
44+
command = mock(InterpreterJupyterKernelSpecCommand);
45+
instance(commandFactory as any).then = undefined;
46+
instance(command as any).then = undefined;
47+
when(
48+
commandFactory.createInterpreterCommand(anything(), anything(), anything(), anything(), anything())
49+
).thenReturn(instance(command));
50+
when(command.exec(anything(), anything())).thenResolve({ stdout: '' });
4851

4952
configuration = new JupyterInterpreterDependencyService(
5053
instance(appShell),
5154
instance(installer),
52-
instance(pythonExecFactory)
55+
instance(commandFactory)
5356
);
5457
});
5558
test('Return ok if all dependencies are installed', async () => {
@@ -91,9 +94,7 @@ suite('Data Science - Jupyter Interpreter Configuration', () => {
9194
// tslint:disable-next-line: no-any
9295
DataScience.jupyterInstall() as any
9396
);
94-
when(pythonExecService.execModule('jupyter', deepEqual(['kernelspec', '--version']), anything())).thenReject(
95-
new Error('Not found')
96-
);
97+
when(command.exec(anything(), anything())).thenReject(new Error('Not found'));
9798
when(installer.install(anything(), anything(), anything())).thenResolve(InstallerResponse.Installed);
9899

99100
const response = await configuration.installMissingDependencies(pythonInterpreter);

0 commit comments

Comments
 (0)