Skip to content

Commit 04587ed

Browse files
authored
When checking if kernelspec subcommand is available, use subprocess (#9400)
* When checking if `kernelspec` subcommand is available, use subprocess * Ignore linter error
1 parent cfe4eb5 commit 04587ed

File tree

3 files changed

+98
-18
lines changed

3 files changed

+98
-18
lines changed
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
import json
5+
import jupyter_client.kernelspec
6+
import sys
7+
8+
9+
specs = jupyter_client.kernelspec.KernelSpecManager().get_all_specs()
10+
all_specs = {
11+
"kernelspecs": specs
12+
}
13+
14+
sys.stdout.write(json.dumps(all_specs))
15+
sys.stdout.flush()

pythonFiles/datascience/jupyter_daemon.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,6 @@ def _print_kernelspec_version(self):
9393
sys.stdout.write(jupyter_client.__version__)
9494
sys.stdout.flush()
9595

96-
def _print_kernelspec_version(self):
97-
import jupyter_client
98-
99-
# Check whether kernelspec module exists.
100-
import jupyter_client.kernelspec
101-
102-
sys.stdout.write(jupyter_client.__version__)
103-
sys.stdout.flush()
104-
10596
def _print_kernel_list(self):
10697
self.log.info("listing kernels")
10798
# Get kernel specs.

src/client/datascience/jupyter/jupyterCommand.ts

Lines changed: 83 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class ProcessJupyterCommand implements IJupyterCommand {
3636
this.interpreterPromise = interpreterService.getInterpreterDetails(this.exe).catch(_e => undefined);
3737
}
3838

39-
public interpreter() : Promise<PythonInterpreter | undefined> {
39+
public interpreter(): Promise<PythonInterpreter | undefined> {
4040
return this.interpreterPromise;
4141
}
4242

@@ -56,7 +56,7 @@ class ProcessJupyterCommand implements IJupyterCommand {
5656
return launcher.exec(this.exe, newArgs, newOptions);
5757
}
5858

59-
private fixupEnv(_env?: NodeJS.ProcessEnv) : Promise<NodeJS.ProcessEnv | undefined> {
59+
private fixupEnv(_env?: NodeJS.ProcessEnv): Promise<NodeJS.ProcessEnv | undefined> {
6060
if (this.activationHelper) {
6161
return this.activationHelper.getActivatedEnvironmentVariables(undefined);
6262
}
@@ -85,18 +85,18 @@ class InterpreterJupyterCommand implements IJupyterCommand {
8585

8686
try {
8787
const output = await svc.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'jupyter_nbInstalled.py')], {});
88-
if (output.stdout.toLowerCase().includes('available')){
88+
if (output.stdout.toLowerCase().includes('available')) {
8989
return svc;
9090
}
91-
} catch (ex){
91+
} catch (ex) {
9292
traceError('Checking whether notebook is importable failed', ex);
9393
}
9494
}
9595
}
96-
return pythonExecutionFactory.createActivatedEnvironment({interpreter: this._interpreter});
96+
return pythonExecutionFactory.createActivatedEnvironment({ interpreter: this._interpreter });
9797
});
9898
}
99-
public interpreter() : Promise<PythonInterpreter | undefined> {
99+
public interpreter(): Promise<PythonInterpreter | undefined> {
100100
return this.interpreterPromise;
101101
}
102102

@@ -134,22 +134,96 @@ export class InterpreterJupyterNotebookCommand extends InterpreterJupyterCommand
134134
}
135135
}
136136

137+
/**
138+
* This class is used to handle kernelspecs.
139+
* I.e. anything to do with the command `python -m jupyter kernelspec`.
140+
*
141+
* @class InterpreterJupyterKernelSpecCommand
142+
* @implements {IJupyterCommand}
143+
*/
144+
// tslint:disable-next-line: max-classes-per-file
145+
export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterCommand {
146+
constructor(moduleName: string, args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter, isActiveInterpreter: boolean) {
147+
super(moduleName, args, pythonExecutionFactory, interpreter, isActiveInterpreter);
148+
}
149+
150+
/**
151+
* Kernelspec subcommand requires special treatment.
152+
* Its possible the sub command hasn't been registered (i.e. jupyter kernelspec command hasn't been installed).
153+
* However its possible the kernlspec modules are available.
154+
* So here's what we have:
155+
* - python -m jupyter kernelspec --version (throws an error, as kernelspect sub command not installed)
156+
* - `import jupyter_client.kernelspec` (works, hence kernelspec modules are available)
157+
* - Problem is daemon will say that `kernelspec` is avaiable, as daemon can work with the `jupyter_client.kernelspec`.
158+
* But rest of extension will assume kernelspec is available and `python -m jupyter kenerlspec --version` will fall over.
159+
* Solution:
160+
* - Run using daemon wrapper code if possible (we don't know whether daemon or python process will run kernel spec).
161+
* - Now, its possible the python daemon process is busy in which case we fall back (in daemon wrapper) to using a python process to run the code.
162+
* - However `python -m jupyter kernelspec` will fall over (as such a sub command hasn't been installed), hence calling daemon code will fail.
163+
* - What we do in such an instance is run the python code `python xyz.py` to deal with kernels.
164+
* If that works, great.
165+
* If that fails, then we know that `kernelspec` sub command doesn't exist and `import jupyter_client.kernelspec` also doesn't work.
166+
* In such a case re-throw the exception from the first execution (possibly the daemon wrapper).
167+
* @param {string[]} args
168+
* @param {SpawnOptions} options
169+
* @returns {Promise<ExecutionResult<string>>}
170+
* @memberof InterpreterJupyterKernelSpecCommand
171+
*/
172+
public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
173+
let exception: Error | undefined;
174+
let output: ExecutionResult<string> = { stdout: '' };
175+
try {
176+
output = await super.exec(args, options);
177+
} catch (ex) {
178+
exception = ex;
179+
}
180+
181+
if (!output.stderr && !exception) {
182+
return output;
183+
}
184+
185+
// We're only interested in `python -m jupyter kernelspec list --json`
186+
const interpreter = await this.interpreter();
187+
if (!interpreter || this.moduleName.toLowerCase() !== 'jupyter' || this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() || args.join(' ').toLowerCase() !== 'list --json') {
188+
if (exception) {
189+
throw exception;
190+
}
191+
return output;
192+
}
193+
try {
194+
// Try getting kernels using python script, if that fails (even if there's output in stderr) rethrow original exception.
195+
const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({ interpreter });
196+
return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')], { ...options, throwOnStdErr: true });
197+
} catch (innerEx) {
198+
traceError('Failed to get a list of the kernelspec using python script', innerEx);
199+
// Rethrow original exception.
200+
if (exception) {
201+
throw exception;
202+
}
203+
return output;
204+
}
205+
}
206+
207+
}
208+
137209
// tslint:disable-next-line: max-classes-per-file
138210
@injectable()
139211
export class JupyterCommandFactory implements IJupyterCommandFactory {
140212

141213
constructor(
142-
@inject(IPythonExecutionFactory) private readonly executionFactory : IPythonExecutionFactory,
143-
@inject(IEnvironmentActivationService) private readonly activationHelper : IEnvironmentActivationService,
214+
@inject(IPythonExecutionFactory) private readonly executionFactory: IPythonExecutionFactory,
215+
@inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService,
144216
@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
145217
@inject(IInterpreterService) private readonly interpreterService: IInterpreterService
146218
) {
147219

148220
}
149221

150222
public createInterpreterCommand(command: JupyterCommands, moduleName: string, args: string[], interpreter: PythonInterpreter, isActiveInterpreter: boolean): IJupyterCommand {
151-
if (command === JupyterCommands.NotebookCommand){
223+
if (command === JupyterCommands.NotebookCommand) {
152224
return new InterpreterJupyterNotebookCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter);
225+
} else if (command === JupyterCommands.KernelSpecCommand) {
226+
return new InterpreterJupyterKernelSpecCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter);
153227
}
154228
return new InterpreterJupyterCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter);
155229
}

0 commit comments

Comments
 (0)