From 0f8a4d8b1eafcd95b8362b9182fb9fcae55b8351 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 6 Jan 2020 10:18:34 -0800 Subject: [PATCH 1/4] When checking if `kernelspec` subcommand is available, use subprocess --- pythonFiles/datascience/jupyter_daemon.py | 24 +++-------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/pythonFiles/datascience/jupyter_daemon.py b/pythonFiles/datascience/jupyter_daemon.py index 24f65548dcd8..6fa2f7dd4a74 100644 --- a/pythonFiles/datascience/jupyter_daemon.py +++ b/pythonFiles/datascience/jupyter_daemon.py @@ -28,13 +28,13 @@ def m_exec_module(self, module_name, args=[], cwd=None, env=None): args = [] if args is None else args if module_name == "jupyter": - if args[0] == "kernelspec" and self._is_module_installed("jupyter_client.kernelspec"): + # When checking if `kernelspec` subcommand is available, use subprocess. + # I.e. just because we can import kernelspec doesn't mean module is installed. + if args[0] == "kernelspec" and self._is_module_installed("jupyter_client.kernelspec") and args[1] != "--version": if args == ["kernelspec", "list", "--json"]: return self._execute_and_capture_output(self._print_kernel_list_json) elif args == ["kernelspec", "list"]: return self._execute_and_capture_output(self._print_kernel_list) - elif args == ["kernelspec", "--version"]: - return self._execute_and_capture_output(self._print_kernelspec_version) if args[0] == "nbconvert" and self._is_module_installed("nbconvert") and args[-1] != "--version": return self._execute_and_capture_output(lambda : self._convert(args)) if args[0] == "notebook" and args[1] == "--version": @@ -84,24 +84,6 @@ def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None): else: return super().m_exec_module_observable(module_name, args, cwd, env) - def _print_kernelspec_version(self): - import jupyter_client - - # Check whether kernelspec module exists. - import jupyter_client.kernelspec - - sys.stdout.write(jupyter_client.__version__) - sys.stdout.flush() - - def _print_kernelspec_version(self): - import jupyter_client - - # Check whether kernelspec module exists. - import jupyter_client.kernelspec - - sys.stdout.write(jupyter_client.__version__) - sys.stdout.flush() - def _print_kernel_list(self): self.log.info("listing kernels") # Get kernel specs. From 4bf1304902cd58226292cf1e61f6e8a34180372d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 6 Jan 2020 11:17:46 -0800 Subject: [PATCH 2/4] Fixes --- pythonFiles/datascience/getJupyterKernels.py | 12 ++++ pythonFiles/datascience/jupyter_daemon.py | 15 ++++- .../datascience/jupyter/jupyterCommand.ts | 60 +++++++++++++++++++ 3 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 pythonFiles/datascience/getJupyterKernels.py diff --git a/pythonFiles/datascience/getJupyterKernels.py b/pythonFiles/datascience/getJupyterKernels.py new file mode 100644 index 000000000000..c4d68584db25 --- /dev/null +++ b/pythonFiles/datascience/getJupyterKernels.py @@ -0,0 +1,12 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +import jupyter_client.kernelspec + +specs = jupyter_client.kernelspec.KernelSpecManager().get_all_specs() +all_specs = { + "kernelspecs": specs +} + +sys.stdout.write(json.dumps(all_specs)) +sys.stdout.flush() diff --git a/pythonFiles/datascience/jupyter_daemon.py b/pythonFiles/datascience/jupyter_daemon.py index 6fa2f7dd4a74..0d2d285de153 100644 --- a/pythonFiles/datascience/jupyter_daemon.py +++ b/pythonFiles/datascience/jupyter_daemon.py @@ -28,13 +28,13 @@ def m_exec_module(self, module_name, args=[], cwd=None, env=None): args = [] if args is None else args if module_name == "jupyter": - # When checking if `kernelspec` subcommand is available, use subprocess. - # I.e. just because we can import kernelspec doesn't mean module is installed. - if args[0] == "kernelspec" and self._is_module_installed("jupyter_client.kernelspec") and args[1] != "--version": + if args[0] == "kernelspec" and self._is_module_installed("jupyter_client.kernelspec"): if args == ["kernelspec", "list", "--json"]: return self._execute_and_capture_output(self._print_kernel_list_json) elif args == ["kernelspec", "list"]: return self._execute_and_capture_output(self._print_kernel_list) + elif args == ["kernelspec", "--version"]: + return self._execute_and_capture_output(self._print_kernelspec_version) if args[0] == "nbconvert" and self._is_module_installed("nbconvert") and args[-1] != "--version": return self._execute_and_capture_output(lambda : self._convert(args)) if args[0] == "notebook" and args[1] == "--version": @@ -84,6 +84,15 @@ def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None): else: return super().m_exec_module_observable(module_name, args, cwd, env) + def _print_kernelspec_version(self): + import jupyter_client + + # Check whether kernelspec module exists. + import jupyter_client.kernelspec + + sys.stdout.write(jupyter_client.__version__) + sys.stdout.flush() + def _print_kernel_list(self): self.log.info("listing kernels") # Get kernel specs. diff --git a/src/client/datascience/jupyter/jupyterCommand.ts b/src/client/datascience/jupyter/jupyterCommand.ts index 889e1d13b9bc..c3eb5147ee83 100644 --- a/src/client/datascience/jupyter/jupyterCommand.ts +++ b/src/client/datascience/jupyter/jupyterCommand.ts @@ -134,6 +134,64 @@ export class InterpreterJupyterNotebookCommand extends InterpreterJupyterCommand } } +/** + * This class is used to handle kernelspecs. + * I.e. anything to do with the command `python -m jupyter kernelspec`. + * + * @class InterpreterJupyterKernelSpecCommand + * @implements {IJupyterCommand} + */ +export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterCommand { + constructor(moduleName: string, args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter, isActiveInterpreter: boolean) { + super(moduleName, args, pythonExecutionFactory, interpreter, isActiveInterpreter); + } + + /** + * Kernelspec subcommand requires special treatment. + * Its possible the sub command hasn't been registered (i.e. jupyter kernelspec command hasn't been installed). + * However its possible the kernlspec modules are available. + * So here's what we have: + * - python -m jupyter kernelspec --version (throws an error, as kernelspect sub command not installed) + * - `import jupyter_client.kernelspec` (works, hence kernelspec modules are available) + * - Problem is daemon will say that `kernelspec` is avaiable, as daemon can work with the `jupyter_client.kernelspec`. + * But rest of extension will assume kernelspec is available and `python -m jupyter kenerlspec --version` will fall over. + * Solution: + * - Run using daemon wrapper code if possible (we don't know whether daemon or python process will run kernel spec). + * - 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. + * - However `python -m jupyter kernelspec` will fall over (as such a sub command hasn't been installed), hence calling daemon code will fail. + * - What we do in such an instance is run the python code `python xyz.py` to deal with kernels. + * If that works, great. + * If that fails, then we know that `kernelspec` sub command doesn't exist and `import jupyter_client.kernelspec` also doesn't work. + * In such a case re-throw the exception from the first execution (possibly the daemon wrapper). + * @param {string[]} args + * @param {SpawnOptions} options + * @returns {Promise>} + * @memberof InterpreterJupyterKernelSpecCommand + */ + public async exec(args: string[], options: SpawnOptions): Promise> { + try { + return super.exec(args, options); + } catch (ex) { + // We're only interested in `python -m jupyter kernelspec list --json` + if (this.moduleName.toLowerCase() !== 'jupyter' || this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() || args.join(' ').toLowerCase() !== 'list --json'){ + throw ex; + } + const interpreter = await this.interpreter(); + if (!interpreter){ + throw ex; + } + try { + const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({interpreter}); + return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles','datascience', 'getJupyterKernels.py')], {...options, throwOnStdErr: true}); + } catch (innerEx){ + traceError('Failed to get a list of the kernelspec using python script', innerEx); + // Rethrow original exception. + throw ex; + } + } + } +} + // tslint:disable-next-line: max-classes-per-file @injectable() export class JupyterCommandFactory implements IJupyterCommandFactory { @@ -150,6 +208,8 @@ export class JupyterCommandFactory implements IJupyterCommandFactory { public createInterpreterCommand(command: JupyterCommands, moduleName: string, args: string[], interpreter: PythonInterpreter, isActiveInterpreter: boolean): IJupyterCommand { if (command === JupyterCommands.NotebookCommand){ return new InterpreterJupyterNotebookCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter); + } else if (command === JupyterCommands.KernelSpecCommand){ + return new InterpreterJupyterKernelSpecCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter); } return new InterpreterJupyterCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter); } From d1c9ddb6921eed72f99c3405d1c6b06c8f680259 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 6 Jan 2020 12:25:02 -0800 Subject: [PATCH 3/4] Fixes --- pythonFiles/datascience/getJupyterKernels.py | 3 + .../datascience/jupyter/jupyterCommand.ts | 63 +++++++++++-------- 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/pythonFiles/datascience/getJupyterKernels.py b/pythonFiles/datascience/getJupyterKernels.py index c4d68584db25..b7bf74850007 100644 --- a/pythonFiles/datascience/getJupyterKernels.py +++ b/pythonFiles/datascience/getJupyterKernels.py @@ -1,7 +1,10 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +import json import jupyter_client.kernelspec +import sys + specs = jupyter_client.kernelspec.KernelSpecManager().get_all_specs() all_specs = { diff --git a/src/client/datascience/jupyter/jupyterCommand.ts b/src/client/datascience/jupyter/jupyterCommand.ts index c3eb5147ee83..8dce0d757e43 100644 --- a/src/client/datascience/jupyter/jupyterCommand.ts +++ b/src/client/datascience/jupyter/jupyterCommand.ts @@ -36,7 +36,7 @@ class ProcessJupyterCommand implements IJupyterCommand { this.interpreterPromise = interpreterService.getInterpreterDetails(this.exe).catch(_e => undefined); } - public interpreter() : Promise { + public interpreter(): Promise { return this.interpreterPromise; } @@ -56,7 +56,7 @@ class ProcessJupyterCommand implements IJupyterCommand { return launcher.exec(this.exe, newArgs, newOptions); } - private fixupEnv(_env?: NodeJS.ProcessEnv) : Promise { + private fixupEnv(_env?: NodeJS.ProcessEnv): Promise { if (this.activationHelper) { return this.activationHelper.getActivatedEnvironmentVariables(undefined); } @@ -85,18 +85,18 @@ class InterpreterJupyterCommand implements IJupyterCommand { try { const output = await svc.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'jupyter_nbInstalled.py')], {}); - if (output.stdout.toLowerCase().includes('available')){ + if (output.stdout.toLowerCase().includes('available')) { return svc; } - } catch (ex){ + } catch (ex) { traceError('Checking whether notebook is importable failed', ex); } } } - return pythonExecutionFactory.createActivatedEnvironment({interpreter: this._interpreter}); + return pythonExecutionFactory.createActivatedEnvironment({ interpreter: this._interpreter }); }); } - public interpreter() : Promise { + public interpreter(): Promise { return this.interpreterPromise; } @@ -169,27 +169,40 @@ export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterComma * @memberof InterpreterJupyterKernelSpecCommand */ public async exec(args: string[], options: SpawnOptions): Promise> { + let exception: Error | undefined; + let output: ExecutionResult = { stdout: '' }; try { - return super.exec(args, options); + output = await super.exec(args, options); } catch (ex) { - // We're only interested in `python -m jupyter kernelspec list --json` - if (this.moduleName.toLowerCase() !== 'jupyter' || this.args.join(' ').toLowerCase() !== `-m jupyter ${JupyterCommands.KernelSpecCommand}`.toLowerCase() || args.join(' ').toLowerCase() !== 'list --json'){ - throw ex; - } - const interpreter = await this.interpreter(); - if (!interpreter){ - throw ex; + exception = ex; + } + + if (!output.stderr && !exception) { + 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') { + if (exception) { + throw exception; } - try { - const activatedEnv = await this.pythonExecutionFactory.createActivatedEnvironment({interpreter}); - return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles','datascience', 'getJupyterKernels.py')], {...options, throwOnStdErr: true}); - } catch (innerEx){ - traceError('Failed to get a list of the kernelspec using python script', innerEx); - // Rethrow original exception. - throw ex; + return output; + } + 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 }); + return activatedEnv.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'getJupyterKernels.py')], { ...options, throwOnStdErr: true }); + } catch (innerEx) { + traceError('Failed to get a list of the kernelspec using python script', innerEx); + // Rethrow original exception. + if (exception) { + throw exception; } + return output; } } + } // tslint:disable-next-line: max-classes-per-file @@ -197,8 +210,8 @@ export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterComma export class JupyterCommandFactory implements IJupyterCommandFactory { constructor( - @inject(IPythonExecutionFactory) private readonly executionFactory : IPythonExecutionFactory, - @inject(IEnvironmentActivationService) private readonly activationHelper : IEnvironmentActivationService, + @inject(IPythonExecutionFactory) private readonly executionFactory: IPythonExecutionFactory, + @inject(IEnvironmentActivationService) private readonly activationHelper: IEnvironmentActivationService, @inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory, @inject(IInterpreterService) private readonly interpreterService: IInterpreterService ) { @@ -206,9 +219,9 @@ export class JupyterCommandFactory implements IJupyterCommandFactory { } public createInterpreterCommand(command: JupyterCommands, moduleName: string, args: string[], interpreter: PythonInterpreter, isActiveInterpreter: boolean): IJupyterCommand { - if (command === JupyterCommands.NotebookCommand){ + if (command === JupyterCommands.NotebookCommand) { return new InterpreterJupyterNotebookCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter); - } else if (command === JupyterCommands.KernelSpecCommand){ + } else if (command === JupyterCommands.KernelSpecCommand) { return new InterpreterJupyterKernelSpecCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter); } return new InterpreterJupyterCommand(moduleName, args, this.executionFactory, interpreter, isActiveInterpreter); From eb2525858e0f60a0b38cd1bf2e6751be7a082afa Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 6 Jan 2020 12:31:49 -0800 Subject: [PATCH 4/4] Ignore linter error --- src/client/datascience/jupyter/jupyterCommand.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/datascience/jupyter/jupyterCommand.ts b/src/client/datascience/jupyter/jupyterCommand.ts index 8dce0d757e43..92c7fd818276 100644 --- a/src/client/datascience/jupyter/jupyterCommand.ts +++ b/src/client/datascience/jupyter/jupyterCommand.ts @@ -141,6 +141,7 @@ export class InterpreterJupyterNotebookCommand extends InterpreterJupyterCommand * @class InterpreterJupyterKernelSpecCommand * @implements {IJupyterCommand} */ +// tslint:disable-next-line: max-classes-per-file export class InterpreterJupyterKernelSpecCommand extends InterpreterJupyterCommand { constructor(moduleName: string, args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter, isActiveInterpreter: boolean) { super(moduleName, args, pythonExecutionFactory, interpreter, isActiveInterpreter);