Start jupyter notebooks even if notebook is not installed in currenv env (#8977)

DonJayamanne · web-flow · commit 7e70cb2d6c6d · 2019-12-10T10:09:32.000-08:00
* Start jupyter notebooks even if notebook is not installed in currenv env

* Increase timeout of test
diff --git a/pythonFiles/datascience/daemon/daemon_python.py b/pythonFiles/datascience/daemon/daemon_python.py
@@ -203,11 +203,14 @@ def m_get_interpreter_information(self):
         }
 
     def m_is_module_installed(self, module_name=None):
+        return {"exists": self._is_module_installed(module_name)}
+
+    def _is_module_installed(self, module_name=None):
         try:
             importlib.import_module(module_name)
-            return {"exists": True}
+            return True
         except Exception:
-            return {"exists": False}
+            return False
 
     @classmethod
     def start_daemon(cls, logging_queue_handler=None):
diff --git a/pythonFiles/datascience/getServerInfo.py b/pythonFiles/datascience/getServerInfo.py
@@ -1,24 +1,31 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License.
 
-from notebook.notebookapp import list_running_servers
-import json
+try:
+    from notebook.notebookapp import list_running_servers
+    import json
 
-server_list = list_running_servers()
+    server_list = list_running_servers()
 
-server_info_list = []
+    server_info_list = []
 
-for si in server_list:
-    server_info_object = {}
-    server_info_object["base_url"] = si['base_url']
-    server_info_object["notebook_dir"] = si['notebook_dir']
-    server_info_object["hostname"] = si['hostname']
-    server_info_object["password"] = si['password']
-    server_info_object["pid"] = si['pid']
-    server_info_object["port"] = si['port']
-    server_info_object["secure"] = si['secure']
-    server_info_object["token"] = si['token']
-    server_info_object["url"] = si['url']
-    server_info_list.append(server_info_object)
+    for si in server_list:
+        server_info_object = {}
+        server_info_object["base_url"] = si['base_url']
+        server_info_object["notebook_dir"] = si['notebook_dir']
+        server_info_object["hostname"] = si['hostname']
+        server_info_object["password"] = si['password']
+        server_info_object["pid"] = si['pid']
+        server_info_object["port"] = si['port']
+        server_info_object["secure"] = si['secure']
+        server_info_object["token"] = si['token']
+        server_info_object["url"] = si['url']
+        server_info_list.append(server_info_object)
 
-print(json.dumps(server_info_list))
+    print(json.dumps(server_info_list))
+except Exception:
+    import subprocess
+    import sys
+    result = subprocess.run(['jupyter', 'notebook', 'list', '--jsonlist'], capture_output=True)
+    encoding = os.getenv('PYTHONIOENCODING', 'utf-8')
+    print(result.stdout.decode(encoding))
diff --git a/pythonFiles/datascience/jupyter_daemon.py b/pythonFiles/datascience/jupyter_daemon.py
@@ -2,9 +2,10 @@
 # Licensed under the MIT License.
 
 import json
-import sys
 import logging
 import os
+import subprocess
+import sys
 from datascience.daemon.daemon_python import (
     error_decorator,
     PythonDaemon as BasePythonDaemon,
@@ -26,18 +27,47 @@ def m_exec_module(self, module_name, args=[], cwd=None, env=None):
         self.log.info("Exec in DS Daemon %s with args %s", module_name, args)
         args = [] if args is None else args
 
-        if module_name == "jupyter" and args == ["kernelspec", "list", "--json"]:
-            return self._execute_and_capture_output(self._print_kernel_list_json)
-        elif module_name == "jupyter" and args == ["kernelspec", "list"]:
-            return self._execute_and_capture_output(self._print_kernel_list)
-        elif module_name == "jupyter" and args == ["kernelspec", "--version"]:
-            return self._execute_and_capture_output(self._print_kernelspec_version)
-        elif module_name == "jupyter" and args[0] == "nbconvert" and args[-1] != "--version":
-            return self._execute_and_capture_output(lambda : self._convert(args))
+        if module_name == "jupyter":
+            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":
+                try:
+                    from notebook import notebookapp as app
+                    return {"stdout": '.'.join(list(str(v) for v in app.version_info))}
+                except Exception:
+                    pass
+            # kernelspec, nbconvert are subcommands of jupyter.
+            # python -m jupyter kernelspec, python -m jupyter nbconvert,
+            # In such cases, even if the modules kernelspec or nbconvert are not installed in the current
+            # environment, jupyter will find them in current path.
+            # So if we cannot find the corresponding subcommands, lets revert to subprocess.
+            self.log.info("Exec in DS Daemon with as subprocess, %s with args %s", module_name, args)
+            return self._exec_with_subprocess(module_name, args, cwd, env)
         else:
             self.log.info("check base class stuff")
             return super().m_exec_module(module_name, args, cwd, env)
 
+    def _exec_with_subprocess(self, module_name, args=[], cwd=None, env=None):
+        # # result = subprocess.run([sys.executable, "-m"] + args, stdout=sys.stdout, stderr=sys.stderr)
+        # return self._execute_and_capture_output(lambda: subprocess.run([sys.executable, "-m", module_name] + args, stdout=sys.stdout, stderr=sys.stderr))
+        result = subprocess.run([sys.executable, "-m", module_name] + args, capture_output=True)
+        encoding = os.getenv('PYTHONIOENCODING', 'utf-8')
+        stdout = result.stdout.decode(encoding)
+        stderr = result.stderr.decode(encoding)
+        self.log.info("subprocess output for, %s with args %s, \nstdout is %s, \nstderr is %s",
+                        module_name, args, stdout, stderr)
+        return {
+            "stdout": stdout,
+            "stderr": stderr
+        }
+
     @error_decorator
     def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None):
         self.log.info("Exec in DS Daemon (observable) %s with args %s", module_name, args)
@@ -50,13 +80,7 @@ def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None):
         if (module_name == "jupyter" and args[0] == "notebook") or (
             module_name == "notebook"
         ):
-            # Args must not have ['notebook'] in the begining. Drop the `notebook` subcommand when using `jupyter`
-            args = args[1:] if args[0] == "notebook" else args
-            self.log.info("Starting notebook with args %s", args)
-
-            # When launching notebook always ensure the first argument is `notebook`.
-            with change_exec_context(args, cwd, env):
-                self._start_notebook(args)
+            self._start_notebook(args, cwd, env)
         else:
             return super().m_exec_module_observable(module_name, args, cwd, env)
 
@@ -107,8 +131,13 @@ def _convert(self, args):
         sys.argv = [""] + args
         app.main()
 
-    def _start_notebook(self, args):
+    def _start_notebook(self, args, cwd, env):
         from notebook import notebookapp as app
 
-        sys.argv = [""] + args
-        app.launch_new_instance()
+        # Args must not have ['notebook'] in the begining. Drop the `notebook` subcommand when using `jupyter`
+        args = args[1:] if args[0] == "notebook" else args
+        self.log.info("Starting notebook with args %s", args)
+
+        # When launching notebook always ensure the first argument is `notebook`.
+        with change_exec_context(args, cwd, env):
+            app.launch_new_instance()
diff --git a/pythonFiles/datascience/jupyter_nbInstalled.py b/pythonFiles/datascience/jupyter_nbInstalled.py
@@ -0,0 +1,8 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+try:
+    from notebook import notebookapp as app
+    print("Available")
+except Exception:
+    print("No")
diff --git a/src/client/datascience/jupyter/jupyterCommand.ts b/src/client/datascience/jupyter/jupyterCommand.ts
@@ -3,7 +3,8 @@
 'use strict';
 import { SpawnOptions } from 'child_process';
 import { inject, injectable } from 'inversify';
-
+import * as path from 'path';
+import { traceError } from '../../common/logger';
 import {
     ExecutionResult,
     IProcessService,
@@ -12,6 +13,7 @@ import {
     IPythonExecutionService,
     ObservableExecutionResult
 } from '../../common/process/types';
+import { EXTENSION_ROOT_DIR } from '../../constants';
 import { IEnvironmentActivationService } from '../../interpreter/activation/types';
 import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
 import { JupyterCommands, PythonDaemonModule } from '../constants';
@@ -71,14 +73,27 @@ class InterpreterJupyterCommand implements IJupyterCommand {
     constructor(protected readonly moduleName: string, protected args: string[], protected readonly pythonExecutionFactory: IPythonExecutionFactory,
         private readonly _interpreter: PythonInterpreter, isActiveInterpreter: boolean) {
         this.interpreterPromise = Promise.resolve(this._interpreter);
-        this.pythonLauncher = this.interpreterPromise.then(interpreter => {
+        this.pythonLauncher = this.interpreterPromise.then(async interpreter => {
             // Create a daemon only if the interpreter is the same as the current interpreter.
-            // We don't want too many daemons (one for each of the users interpreter on their machine).
+            // We don't want too many daemons (we don't want one for each of the users interpreter on their machine).
             if (isActiveInterpreter) {
-                return pythonExecutionFactory.createDaemon({ daemonModule: PythonDaemonModule, pythonPath: interpreter!.path });
-            } else {
-                return pythonExecutionFactory.createActivatedEnvironment({interpreter: this._interpreter});
+                const svc = await pythonExecutionFactory.createDaemon({ daemonModule: PythonDaemonModule, pythonPath: interpreter!.path });
+
+                // If we're using this command to start notebook, then ensure the daemon can start a notebook inside it.
+                if ((moduleName.toLowerCase() === 'jupyter' && args.join(' ').toLowerCase().startsWith('-m jupyter notebook')) ||
+                    (moduleName.toLowerCase() === 'notebook' && args.join(' ').toLowerCase().startsWith('-m notebook'))) {
+
+                    try {
+                        const output = await svc.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'jupyter_nbInstalled.py')], {});
+                        if (output.stdout.toLowerCase().includes('available')){
+                            return svc;
+                        }
+                    } catch (ex){
+                        traceError('Checking whether notebook is importable failed', ex);
+                    }
+                }
             }
+            return pythonExecutionFactory.createActivatedEnvironment({interpreter: this._interpreter});
         });
     }
     public interpreter() : Promise<PythonInterpreter | undefined> {
diff --git a/src/client/datascience/jupyter/kernels/kernelService.ts b/src/client/datascience/jupyter/kernels/kernelService.ts
@@ -16,6 +16,7 @@ import { traceDecorators, traceError, traceInfo, traceVerbose, traceWarning } fr
 import { IFileSystem } from '../../../common/platform/types';
 import { IPythonExecutionFactory } from '../../../common/process/types';
 import { IInstaller, InstallerResponse, Product, ReadWrite } from '../../../common/types';
+import { sleep } from '../../../common/utils/async';
 import { noop } from '../../../common/utils/misc';
 import { IEnvironmentActivationService } from '../../../interpreter/activation/types';
 import { IInterpreterService, PythonInterpreter } from '../../../interpreter/contracts';
@@ -250,9 +251,18 @@ export class KernelService {
             return;
         }
 
-        const kernel = await this.findMatchingKernelSpec({ display_name: interpreter.displayName, name }, undefined, cancelToken);
-        if (Cancellation.isCanceled(cancelToken)) {
-            return;
+        let kernel = await this.findMatchingKernelSpec({ display_name: interpreter.displayName, name }, undefined, cancelToken);
+        for (let counter = 0; counter < 5; counter += 1){
+            if (Cancellation.isCanceled(cancelToken)) {
+                return;
+            }
+            if (kernel){
+                break;
+            }
+            traceWarning('Waiting for 500ms for registered kernel to get detected');
+            // Wait for jupyter server to get updated with the new kernel information.
+            await sleep(500);
+            kernel = await this.findMatchingKernelSpec({ display_name: interpreter.displayName, name }, undefined, cancelToken);
         }
         if (!kernel) {
             const error = `Kernel not created with the name ${name}, display_name ${interpreter.displayName}. Output is ${output.stdout}`;
diff --git a/src/test/datascience/jupyter/kernels/kernelService.unit.test.ts b/src/test/datascience/jupyter/kernels/kernelService.unit.test.ts
@@ -275,7 +275,7 @@ suite('Data Science - KernelService', () => {
             const kernelName = installArgs[3];
             assert.deepEqual(installArgs, ['install', '--user', '--name', kernelName, '--display-name', interpreter.displayName]);
             await assert.isRejected(promise, `Kernel not created with the name ${kernelName}, display_name ${interpreter.displayName}. Output is `);
-        });
+        }).timeout(5_000);
         test('Fail if installed kernel is not an instance of JupyterKernelSpec', async () => {
             when(execService.execModule('ipykernel', anything(), anything())).thenResolve({ stdout: '' });
             when(installer.isInstalled(Product.ipykernel, interpreter)).thenResolve(true);
