Skip to content

Commit c027287

Browse files
DonJayamanneDonJayamanne
authored
Fixes to Jupyter daemon and refactor jupyter commands (microsoft#8469)
* Fixes to Jupyter daemon and refactor jupyter commands * Fix compiler issues * Fix linter * Fixes to tests
1 parent 9862c6a commit c027287

7 files changed

Lines changed: 129 additions & 57 deletions

File tree

‎pythonFiles/datascience/jupyter_daemon.py‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from datascience.daemon.daemon_python import (
88
error_decorator,
99
PythonDaemon as BasePythonDaemon,
10+
change_exec_context,
1011
)
1112

1213
log = logging.getLogger(__name__)
@@ -24,7 +25,7 @@ def __getitem__(self, item):
2425

2526
@error_decorator
2627
def m_exec_module(self, module_name, args=[], cwd=None, env=None):
27-
log.info("Exec in child class %s with args %s", module_name, args)
28+
log.info("Exec in DS Daemon %s with args %s", module_name, args)
2829
args = [] if args is None else args
2930

3031
if module_name == "jupyter" and args == ["kernelspec", "list"]:
@@ -35,6 +36,37 @@ def m_exec_module(self, module_name, args=[], cwd=None, env=None):
3536
log.info("check base class stuff")
3637
return super().m_exec_module(module_name, args, cwd, env)
3738

39+
@error_decorator
40+
def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None):
41+
log.info("Exec in DS Daemon (observable) %s with args %s", module_name, args)
42+
args = [] if args is None else args
43+
44+
# Assumption is that `python -m jupyter notebook` or `python -m notebook` with observable output
45+
# will only ever be used to start a notebook and nothing else.
46+
# E.g. `python -m jupyter notebook --version` wouldn't require the use of exec_module_observable,
47+
# In such cases, we can get the output immediately.
48+
if (module_name == "jupyter" and args[0] == "notebook") or (
49+
module_name == "notebook"
50+
):
51+
# Args must not have ['notebook'] in the begining. Drop the `notebook` subcommand when using `jupyter`
52+
args = args[1:] if args[0] == "notebook" else args
53+
log.info("Starting notebook with args %s", args)
54+
55+
# When launching notebook always ensure the first argument is `notebook`.
56+
with change_exec_context(args, cwd, env):
57+
self._start_notebook(args)
58+
else:
59+
return super().m_exec_module_observable(module_name, args, cwd, env)
60+
61+
def _print_kernelspec_version(self):
62+
import jupyter_client
63+
64+
# Check whether kernelspec module exists.
65+
import jupyter_client.kernelspec
66+
67+
sys.stdout.write(jupyter_client.__version__)
68+
sys.stdout.flush()
69+
3870
def _print_kernelspec_version(self):
3971
import jupyter_client
4072

@@ -55,9 +87,8 @@ def _print_kernel_list(self):
5587
)
5688
sys.stdout.flush()
5789

58-
def m_hello(self, rootUri=None, **kwargs):
59-
from notebook.notebookapp import main
90+
def _start_notebook(self, args):
91+
from notebook import notebookapp as app
6092

61-
sys.argv = ["notebook", "--no-browser"]
62-
main()
63-
return {}
93+
sys.argv = [""] + args
94+
app.launch_new_instance()

‎src/client/common/process/pythonDaemon.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,14 +199,19 @@ export class PythonDaemonExecutionService implements IPythonDaemonExecutionServi
199199
const subject = new Subject<Output<string>>();
200200
const start = async () => {
201201
type ExecResponse = ErrorResponse & { stdout: string; stderr?: string };
202+
let response: ExecResponse;
202203
if ('fileName' in moduleOrFile) {
203204
// tslint:disable-next-line: no-any
204205
const request = new RequestType<{ file_name: string; args: string[]; cwd?: string; env?: any }, ExecResponse, void, void>('exec_file_observable');
205-
await this.connection.sendRequest(request, { file_name: moduleOrFile.fileName, args, cwd: options.cwd, env: options.env });
206+
response = await this.connection.sendRequest(request, { file_name: moduleOrFile.fileName, args, cwd: options.cwd, env: options.env });
206207
} else {
207208
// tslint:disable-next-line: no-any
208209
const request = new RequestType<{ module_name: string; args: string[]; cwd?: string; env?: any }, ExecResponse, void, void>('exec_module_observable');
209-
await this.connection.sendRequest(request, { module_name: moduleOrFile.moduleName, args, cwd: options.cwd, env: options.env });
210+
response = await this.connection.sendRequest(request, { module_name: moduleOrFile.moduleName, args, cwd: options.cwd, env: options.env });
211+
}
212+
// Might not get a response object back, as its observable.
213+
if (response && response.error){
214+
throw new StdErrError(response.error);
210215
}
211216
};
212217
let stdErr = '';

‎src/client/common/process/pythonExecutionFactory.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { traceError } from '../logger';
1515
import { IConfigurationService, IDisposableRegistry } from '../types';
1616
import { sleep } from '../utils/async';
1717
import { ProcessService } from './proc';
18+
import { PythonDaemonExecutionService } from './pythonDaemon';
1819
import { PythonExecutionService } from './pythonProcess';
1920
import {
2021
DaemonExecutionFactoryCreationOptions,
@@ -67,7 +68,8 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
6768
env.PYTHONPATH = envPythonPath;
6869
}
6970
env.PYTHONUNBUFFERED = '1';
70-
const daemonProc = activatedProc!.execModuleObservable('datascience.daemon', [`--daemon-module=${options.daemonModule}`], { env });
71+
const args = options.daemonModule ? [`--daemon-module=${options.daemonModule}`] : [];
72+
const daemonProc = activatedProc!.execModuleObservable('datascience.daemon', args, { env });
7173
if (!daemonProc.proc) {
7274
throw new Error('Failed to create Daemon Proc');
7375
}
@@ -92,7 +94,8 @@ export class PythonExecutionFactory implements IPythonExecutionFactory {
9294
if (result.pong !== 'hello') {
9395
throw new Error(`Daemon did not reply to the ping, received: ${result.pong}`);
9496
}
95-
return new options.daemonClass(activatedProc, pythonPath, daemonProc.proc, connection);
97+
const cls = options.daemonClass ? options.daemonClass : PythonDaemonExecutionService;
98+
return new cls(activatedProc!, pythonPath, daemonProc.proc, connection);
9699
} catch (ex) {
97100
traceError('Failed to start the Daemon, StdErr: ', stdError);
98101
traceError('Failed to start the Daemon, ProcEndEx', procEndEx || ex);

‎src/client/common/process/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ export type DaemonExecutionFactoryCreationOptions = ExecutionFactoryCreationOpti
7070
*
7171
* @type {string}
7272
*/
73-
daemonModule: string;
74-
daemonClass: Newable<IPythonDaemonExecutionService>;
73+
daemonModule?: string;
74+
daemonClass?: Newable<IPythonDaemonExecutionService>;
7575
};
7676
export type ExecutionFactoryCreateWithEnvironmentOptions = {
7777
resource?: Uri;

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

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '../../common/process/types';
1515
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
1616
import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
17+
import { JupyterCommands } from '../constants';
1718
import { IJupyterCommand, IJupyterCommandFactory } from '../types';
1819

1920
// 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 {
6465
}
6566

6667
class InterpreterJupyterCommand implements IJupyterCommand {
67-
private requiredArgs: string[];
68-
private interpreterPromise: Promise<PythonInterpreter | undefined>;
68+
protected interpreterPromise: Promise<PythonInterpreter | undefined>;
6969
private pythonLauncher: Promise<IPythonExecutionService>;
7070

71-
constructor(args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter) {
72-
this.requiredArgs = args;
73-
this.interpreterPromise = Promise.resolve(interpreter);
74-
this.pythonLauncher = pythonExecutionFactory.createActivatedEnvironment({ resource: undefined, interpreter, allowEnvironmentFetchExceptions: true });
71+
constructor(protected readonly moduleName: string, protected args: string[], pythonExecutionFactory: IPythonExecutionFactory, private readonly _interpreter: PythonInterpreter) {
72+
this.interpreterPromise = Promise.resolve(this._interpreter);
73+
this.pythonLauncher = pythonExecutionFactory.createActivatedEnvironment({ resource: undefined, interpreter: _interpreter, allowEnvironmentFetchExceptions: true });
7574
}
7675
public interpreter() : Promise<PythonInterpreter | undefined> {
7776
return this.interpreterPromise;
@@ -80,18 +79,32 @@ class InterpreterJupyterCommand implements IJupyterCommand {
8079
public async execObservable(args: string[], options: SpawnOptions): Promise<ObservableExecutionResult<string>> {
8180
const newOptions = { ...options };
8281
const launcher = await this.pythonLauncher;
83-
const newArgs = [...this.requiredArgs, ...args];
82+
const newArgs = [...this.args, ...args];
8483
return launcher.execObservable(newArgs, newOptions);
8584
}
8685

8786
public async exec(args: string[], options: SpawnOptions): Promise<ExecutionResult<string>> {
8887
const newOptions = { ...options };
8988
const launcher = await this.pythonLauncher;
90-
const newArgs = [...this.requiredArgs, ...args];
89+
const newArgs = [...this.args, ...args];
9190
return launcher.exec(newArgs, newOptions);
9291
}
9392
}
9493

94+
/**
95+
* This class is used to launch the notebook.
96+
* I.e. anything to do with the command `python -m jupyter notebook` or `python -m notebook`.
97+
*
98+
* @class InterpreterJupyterNotebookCommand
99+
* @implements {IJupyterCommand}
100+
*/
101+
class InterpreterJupyterNotebookCommand extends InterpreterJupyterCommand {
102+
constructor(moduleName: string, args: string[], pythonExecutionFactory: IPythonExecutionFactory, interpreter: PythonInterpreter) {
103+
super(moduleName, args, pythonExecutionFactory, interpreter);
104+
}
105+
}
106+
107+
// tslint:disable-next-line: max-classes-per-file
95108
@injectable()
96109
export class JupyterCommandFactory implements IJupyterCommandFactory {
97110

@@ -104,8 +117,11 @@ export class JupyterCommandFactory implements IJupyterCommandFactory {
104117

105118
}
106119

107-
public createInterpreterCommand(args: string[], interpreter: PythonInterpreter): IJupyterCommand {
108-
return new InterpreterJupyterCommand(args, this.executionFactory, interpreter);
120+
public createInterpreterCommand(command: JupyterCommands, moduleName: string, args: string[], interpreter: PythonInterpreter): IJupyterCommand {
121+
if (command === JupyterCommands.NotebookCommand){
122+
return new InterpreterJupyterNotebookCommand(moduleName, args, this.executionFactory, interpreter);
123+
}
124+
return new InterpreterJupyterCommand(moduleName, args, this.executionFactory, interpreter);
109125
}
110126

111127
public createProcessCommand(exe: string, args: string[]): IJupyterCommand {

0 commit comments

Comments
 (0)