Fixes to Jupyter daemon and refactor jupyter commands (#8469)

* Fixes to Jupyter daemon and refactor jupyter commands
* Fix compiler issues
* Fix linter
* Fixes to tests

Author: DonJayamanne

pythonFiles/datascience/jupyter_daemon.py

@@ -7,6 +7,7 @@
 from datascience.daemon.daemon_python import (
     error_decorator,
     PythonDaemon as BasePythonDaemon,
+    change_exec_context,
 )
 
 log = logging.getLogger(__name__)
@@ -24,7 +25,7 @@ def __getitem__(self, item):
 
     @error_decorator
     def m_exec_module(self, module_name, args=[], cwd=None, env=None):
-        log.info("Exec in child class %s with args %s", module_name, args)
+        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"]:
@@ -35,6 +36,37 @@ def m_exec_module(self, module_name, args=[], cwd=None, env=None):
             log.info("check base class stuff")
             return super().m_exec_module(module_name, args, cwd, env)
 
+    @error_decorator
+    def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None):
+        log.info("Exec in DS Daemon (observable) %s with args %s", module_name, args)
+        args = [] if args is None else args
+
+        # Assumption is that `python -m jupyter notebook` or `python -m notebook` with observable output
+        # will only ever be used to start a notebook and nothing else.
+        # E.g. `python -m jupyter notebook --version` wouldn't require the use of exec_module_observable,
+        # In such cases, we can get the output immediately.
+        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
+            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)
+        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
 
@@ -55,9 +87,8 @@ def _print_kernel_list(self):
         )
         sys.stdout.flush()
 
-    def m_hello(self, rootUri=None, **kwargs):
-        from notebook.notebookapp import main
+    def _start_notebook(self, args):
+        from notebook import notebookapp as app
 
-        sys.argv = ["notebook", "--no-browser"]
-        main()
-        return {}
+        sys.argv = [""] + args
+        app.launch_new_instance()

src/client/common/process/pythonDaemon.ts

@@ -199,14 +199,19 @@ export class PythonDaemonExecutionService implements IPythonDaemonExecutionServi
         const subject = new Subject<Output<string>>();
         const start = async () => {
             type ExecResponse = ErrorResponse & { stdout: string; stderr?: string };
+            let response: ExecResponse;
             if ('fileName' in moduleOrFile) {
                 // tslint:disable-next-line: no-any
                 const request = new RequestType<{ file_name: string; args: string[]; cwd?: string; env?: any }, ExecResponse, void, void>('exec_file_observable');
-                await this.connection.sendRequest(request, { file_name: moduleOrFile.fileName, args, cwd: options.cwd, env: options.env });
+                response = await this.connection.sendRequest(request, { file_name: moduleOrFile.fileName, args, cwd: options.cwd, env: options.env });
             } else {
                 // tslint:disable-next-line: no-any
                 const request = new RequestType<{ module_name: string; args: string[]; cwd?: string; env?: any }, ExecResponse, void, void>('exec_module_observable');
-                await this.connection.sendRequest(request, { module_name: moduleOrFile.moduleName, args, cwd: options.cwd, env: options.env });
+                response = await this.connection.sendRequest(request, { module_name: moduleOrFile.moduleName, args, cwd: options.cwd, env: options.env });
+            }
+            // Might not get a response object back, as its observable.
+            if (response && response.error){
+                throw new StdErrError(response.error);
             }
         };
         let stdErr = '';

src/client/common/process/pythonExecutionFactory.ts

@@ -15,6 +15,7 @@ import { traceError } from '../logger';
 import { IConfigurationService, IDisposableRegistry } from '../types';
 import { sleep } from '../utils/async';
 import { ProcessService } from './proc';
+import { PythonDaemonExecutionService } from './pythonDaemon';
 import { PythonExecutionService } from './pythonProcess';
 import {
     DaemonExecutionFactoryCreationOptions,
@@ -67,7 +68,8 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
             env.PYTHONPATH = envPythonPath;
         }
         env.PYTHONUNBUFFERED = '1';
-        const daemonProc = activatedProc!.execModuleObservable('datascience.daemon', [`--daemon-module=${options.daemonModule}`], { env });
+        const args = options.daemonModule ? [`--daemon-module=${options.daemonModule}`] : [];
+        const daemonProc = activatedProc!.execModuleObservable('datascience.daemon', args, { env });
         if (!daemonProc.proc) {
             throw new Error('Failed to create Daemon Proc');
         }
@@ -92,7 +94,8 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
             if (result.pong !== 'hello') {
                 throw new Error(`Daemon did not reply to the ping, received: ${result.pong}`);
             }
-            return new options.daemonClass(activatedProc, pythonPath, daemonProc.proc, connection);
+            const cls = options.daemonClass ? options.daemonClass : PythonDaemonExecutionService;
+            return new cls(activatedProc!, pythonPath, daemonProc.proc, connection);
         } catch (ex) {
             traceError('Failed to start the Daemon, StdErr: ', stdError);
             traceError('Failed to start the Daemon, ProcEndEx', procEndEx || ex);

src/client/common/process/types.ts

@@ -70,8 +70,8 @@ export type DaemonExecutionFactoryCreationOptions = ExecutionFactoryCreationOpti
      *
      * @type {string}
      */
-    daemonModule: string;
-    daemonClass: Newable<IPythonDaemonExecutionService>;
+    daemonModule?: string;
+    daemonClass?: Newable<IPythonDaemonExecutionService>;
 };
 export type ExecutionFactoryCreateWithEnvironmentOptions = {
     resource?: Uri;

src/client/datascience/jupyter/jupyterCommand.ts

@@ -14,6 +14,7 @@ import {
 } from '../../common/process/types';
 import { IEnvironmentActivationService } from '../../interpreter/activation/types';
 import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
+import { JupyterCommands } from '../constants';
 import { IJupyterCommand, IJupyterCommandFactory } from '../types';
 
 // JupyterCommand objects represent some process that can be launched that should be guaranteed to work because it
@@ -64,14 +65,12 @@ class ProcessJupyterCommand implements IJupyterCommand {
 }
 
 class InterpreterJupyterCommand implements IJupyterCommand {
-    private requiredArgs: string[];
-    private interpreterPromise: Promise<PythonInterpreter | undefined>;
+    protected interpreterPromise: Promise<PythonInterpreter | undefined>;
     private pythonLauncher: Promise<IPythonExecutionService>;
 
-    constructor(args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter) {
-        this.requiredArgs = args;
-        this.interpreterPromise = Promise.resolve(interpreter);
-        this.pythonLauncher = pythonExecutionFactory.createActivatedEnvironment({ resource: undefined, interpreter, allowEnvironmentFetchExceptions: true });
+    constructor(protected readonly moduleName: string, protected args: string[], pythonExecutionFactory: IPythonExecutionFactory, private readonly _interpreter: PythonInterpreter) {
+        this.interpreterPromise = Promise.resolve(this._interpreter);
+        this.pythonLauncher = pythonExecutionFactory.createActivatedEnvironment({ resource: undefined, interpreter: _interpreter, allowEnvironmentFetchExceptions: true });
     }
     public interpreter() : Promise<PythonInterpreter | undefined> {
         return this.interpreterPromise;
@@ -80,18 +79,32 @@ class InterpreterJupyterCommand implements IJupyterCommand {
     public async execObservable(args: string[], options: SpawnOptions): Promise<ObservableExecutionResult<string>> {
         const newOptions = { ...options };
         const launcher = await this.pythonLauncher;
-        const newArgs = [...this.requiredArgs, ...args];
+        const newArgs = [...this.args, ...args];
         return launcher.execObservable(newArgs, newOptions);
     }
 
     public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
         const newOptions = { ...options };
         const launcher = await this.pythonLauncher;
-        const newArgs = [...this.requiredArgs, ...args];
+        const newArgs = [...this.args, ...args];
         return launcher.exec(newArgs, newOptions);
     }
 }
 
+/**
+ * This class is used to launch the notebook.
+ * I.e. anything to do with the command `python -m jupyter notebook` or `python -m notebook`.
+ *
+ * @class InterpreterJupyterNotebookCommand
+ * @implements {IJupyterCommand}
+ */
+class InterpreterJupyterNotebookCommand extends InterpreterJupyterCommand {
+    constructor(moduleName: string, args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter) {
+        super(moduleName, args, pythonExecutionFactory, interpreter);
+    }
+}
+
+// tslint:disable-next-line: max-classes-per-file
 @injectable()
 export class JupyterCommandFactory implements IJupyterCommandFactory {
 
@@ -104,8 +117,11 @@ export class JupyterCommandFactory implements IJupyterCommandFactory {
 
     }
 
-    public createInterpreterCommand(args: string[], interpreter: PythonInterpreter): IJupyterCommand {
-        return new InterpreterJupyterCommand(args, this.executionFactory, interpreter);
+    public createInterpreterCommand(command: JupyterCommands, moduleName: string, args: string[], interpreter: PythonInterpreter): IJupyterCommand {
+        if (command === JupyterCommands.NotebookCommand){
+            return new InterpreterJupyterNotebookCommand(moduleName, args, this.executionFactory, interpreter);
+        }
+        return new InterpreterJupyterCommand(moduleName, args, this.executionFactory, interpreter);
     }
 
     public createProcessCommand(exe: string, args: string[]): IJupyterCommand {