Skip to content

Commit 7e70cb2

Browse files
DonJayamanneDonJayamanne
authored
Start jupyter notebooks even if notebook is not installed in currenv env (#8977)
* Start jupyter notebooks even if notebook is not installed in currenv env * Increase timeout of test
1 parent b96e7a3 commit 7e70cb2

7 files changed

Lines changed: 120 additions & 48 deletions

File tree

‎pythonFiles/datascience/daemon/daemon_python.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,14 @@ def m_get_interpreter_information(self):
203203
}
204204

205205
def m_is_module_installed(self, module_name=None):
206+
return {"exists": self._is_module_installed(module_name)}
207+
208+
def _is_module_installed(self, module_name=None):
206209
try:
207210
importlib.import_module(module_name)
208-
return {"exists": True}
211+
return True
209212
except Exception:
210-
return {"exists": False}
213+
return False
211214

212215
@classmethod
213216
def start_daemon(cls, logging_queue_handler=None):

‎pythonFiles/datascience/getServerInfo.py‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,31 @@
11
# Copyright (c) Microsoft Corporation. All rights reserved.
22
# Licensed under the MIT License.
33

4-
from notebook.notebookapp import list_running_servers
5-
import json
4+
try:
5+
from notebook.notebookapp import list_running_servers
6+
import json
67

7-
server_list = list_running_servers()
8+
server_list = list_running_servers()
89

9-
server_info_list = []
10+
server_info_list = []
1011

11-
for si in server_list:
12-
server_info_object = {}
13-
server_info_object["base_url"] = si['base_url']
14-
server_info_object["notebook_dir"] = si['notebook_dir']
15-
server_info_object["hostname"] = si['hostname']
16-
server_info_object["password"] = si['password']
17-
server_info_object["pid"] = si['pid']
18-
server_info_object["port"] = si['port']
19-
server_info_object["secure"] = si['secure']
20-
server_info_object["token"] = si['token']
21-
server_info_object["url"] = si['url']
22-
server_info_list.append(server_info_object)
12+
for si in server_list:
13+
server_info_object = {}
14+
server_info_object["base_url"] = si['base_url']
15+
server_info_object["notebook_dir"] = si['notebook_dir']
16+
server_info_object["hostname"] = si['hostname']
17+
server_info_object["password"] = si['password']
18+
server_info_object["pid"] = si['pid']
19+
server_info_object["port"] = si['port']
20+
server_info_object["secure"] = si['secure']
21+
server_info_object["token"] = si['token']
22+
server_info_object["url"] = si['url']
23+
server_info_list.append(server_info_object)
2324

24-
print(json.dumps(server_info_list))
25+
print(json.dumps(server_info_list))
26+
except Exception:
27+
import subprocess
28+
import sys
29+
result = subprocess.run(['jupyter', 'notebook', 'list', '--jsonlist'], capture_output=True)
30+
encoding = os.getenv('PYTHONIOENCODING', 'utf-8')
31+
print(result.stdout.decode(encoding))

‎pythonFiles/datascience/jupyter_daemon.py‎

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
# Licensed under the MIT License.
33

44
import json
5-
import sys
65
import logging
76
import os
7+
import subprocess
8+
import sys
89
from datascience.daemon.daemon_python import (
910
error_decorator,
1011
PythonDaemon as BasePythonDaemon,
@@ -26,18 +27,47 @@ def m_exec_module(self, module_name, args=[], cwd=None, env=None):
2627
self.log.info("Exec in DS Daemon %s with args %s", module_name, args)
2728
args = [] if args is None else args
2829

29-
if module_name == "jupyter" and args == ["kernelspec", "list", "--json"]:
30-
return self._execute_and_capture_output(self._print_kernel_list_json)
31-
elif module_name == "jupyter" and args == ["kernelspec", "list"]:
32-
return self._execute_and_capture_output(self._print_kernel_list)
33-
elif module_name == "jupyter" and args == ["kernelspec", "--version"]:
34-
return self._execute_and_capture_output(self._print_kernelspec_version)
35-
elif module_name == "jupyter" and args[0] == "nbconvert" and args[-1] != "--version":
36-
return self._execute_and_capture_output(lambda : self._convert(args))
30+
if module_name == "jupyter":
31+
if args[0] == "kernelspec" and self._is_module_installed("jupyter_client.kernelspec"):
32+
if args == ["kernelspec", "list", "--json"]:
33+
return self._execute_and_capture_output(self._print_kernel_list_json)
34+
elif args == ["kernelspec", "list"]:
35+
return self._execute_and_capture_output(self._print_kernel_list)
36+
elif args == ["kernelspec", "--version"]:
37+
return self._execute_and_capture_output(self._print_kernelspec_version)
38+
if args[0] == "nbconvert" and self._is_module_installed("nbconvert") and args[-1] != "--version":
39+
return self._execute_and_capture_output(lambda : self._convert(args))
40+
if args[0] == "notebook" and args[1] == "--version":
41+
try:
42+
from notebook import notebookapp as app
43+
return {"stdout": '.'.join(list(str(v) for v in app.version_info))}
44+
except Exception:
45+
pass
46+
# kernelspec, nbconvert are subcommands of jupyter.
47+
# python -m jupyter kernelspec, python -m jupyter nbconvert,
48+
# In such cases, even if the modules kernelspec or nbconvert are not installed in the current
49+
# environment, jupyter will find them in current path.
50+
# So if we cannot find the corresponding subcommands, lets revert to subprocess.
51+
self.log.info("Exec in DS Daemon with as subprocess, %s with args %s", module_name, args)
52+
return self._exec_with_subprocess(module_name, args, cwd, env)
3753
else:
3854
self.log.info("check base class stuff")
3955
return super().m_exec_module(module_name, args, cwd, env)
4056

57+
def _exec_with_subprocess(self, module_name, args=[], cwd=None, env=None):
58+
# # result = subprocess.run([sys.executable, "-m"] + args, stdout=sys.stdout, stderr=sys.stderr)
59+
# return self._execute_and_capture_output(lambda: subprocess.run([sys.executable, "-m", module_name] + args, stdout=sys.stdout, stderr=sys.stderr))
60+
result = subprocess.run([sys.executable, "-m", module_name] + args, capture_output=True)
61+
encoding = os.getenv('PYTHONIOENCODING', 'utf-8')
62+
stdout = result.stdout.decode(encoding)
63+
stderr = result.stderr.decode(encoding)
64+
self.log.info("subprocess output for, %s with args %s, \nstdout is %s, \nstderr is %s",
65+
module_name, args, stdout, stderr)
66+
return {
67+
"stdout": stdout,
68+
"stderr": stderr
69+
}
70+
4171
@error_decorator
4272
def m_exec_module_observable(self, module_name, args=None, cwd=None, env=None):
4373
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):
5080
if (module_name == "jupyter" and args[0] == "notebook") or (
5181
module_name == "notebook"
5282
):
53-
# Args must not have ['notebook'] in the begining. Drop the `notebook` subcommand when using `jupyter`
54-
args = args[1:] if args[0] == "notebook" else args
55-
self.log.info("Starting notebook with args %s", args)
56-
57-
# When launching notebook always ensure the first argument is `notebook`.
58-
with change_exec_context(args, cwd, env):
59-
self._start_notebook(args)
83+
self._start_notebook(args, cwd, env)
6084
else:
6185
return super().m_exec_module_observable(module_name, args, cwd, env)
6286

@@ -107,8 +131,13 @@ def _convert(self, args):
107131
sys.argv = [""] + args
108132
app.main()
109133

110-
def _start_notebook(self, args):
134+
def _start_notebook(self, args, cwd, env):
111135
from notebook import notebookapp as app
112136

113-
sys.argv = [""] + args
114-
app.launch_new_instance()
137+
# Args must not have ['notebook'] in the begining. Drop the `notebook` subcommand when using `jupyter`
138+
args = args[1:] if args[0] == "notebook" else args
139+
self.log.info("Starting notebook with args %s", args)
140+
141+
# When launching notebook always ensure the first argument is `notebook`.
142+
with change_exec_context(args, cwd, env):
143+
app.launch_new_instance()

‎pythonFiles/datascience/jupyter_nbInstalled.py‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
try:
5+
from notebook import notebookapp as app
6+
print("Available")
7+
except Exception:
8+
print("No")

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

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
'use strict';
44
import { SpawnOptions } from 'child_process';
55
import { inject, injectable } from 'inversify';
6-
6+
import * as path from 'path';
7+
import { traceError } from '../../common/logger';
78
import {
89
ExecutionResult,
910
IProcessService,
@@ -12,6 +13,7 @@ import {
1213
IPythonExecutionService,
1314
ObservableExecutionResult
1415
} from '../../common/process/types';
16+
import { EXTENSION_ROOT_DIR } from '../../constants';
1517
import { IEnvironmentActivationService } from '../../interpreter/activation/types';
1618
import { IInterpreterService, PythonInterpreter } from '../../interpreter/contracts';
1719
import { JupyterCommands, PythonDaemonModule } from '../constants';
@@ -71,14 +73,27 @@ class InterpreterJupyterCommand implements IJupyterCommand {
7173
constructor(protected readonly moduleName: string, protected args: string[], protected readonly pythonExecutionFactory: IPythonExecutionFactory,
7274
private readonly _interpreter: PythonInterpreter, isActiveInterpreter: boolean) {
7375
this.interpreterPromise = Promise.resolve(this._interpreter);
74-
this.pythonLauncher = this.interpreterPromise.then(interpreter => {
76+
this.pythonLauncher = this.interpreterPromise.then(async interpreter => {
7577
// Create a daemon only if the interpreter is the same as the current interpreter.
76-
// We don't want too many daemons (one for each of the users interpreter on their machine).
78+
// We don't want too many daemons (we don't want one for each of the users interpreter on their machine).
7779
if (isActiveInterpreter) {
78-
return pythonExecutionFactory.createDaemon({ daemonModule: PythonDaemonModule, pythonPath: interpreter!.path });
79-
} else {
80-
return pythonExecutionFactory.createActivatedEnvironment({interpreter: this._interpreter});
80+
const svc = await pythonExecutionFactory.createDaemon({ daemonModule: PythonDaemonModule, pythonPath: interpreter!.path });
81+
82+
// If we're using this command to start notebook, then ensure the daemon can start a notebook inside it.
83+
if ((moduleName.toLowerCase() === 'jupyter' && args.join(' ').toLowerCase().startsWith('-m jupyter notebook')) ||
84+
(moduleName.toLowerCase() === 'notebook' && args.join(' ').toLowerCase().startsWith('-m notebook'))) {
85+
86+
try {
87+
const output = await svc.exec([path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'datascience', 'jupyter_nbInstalled.py')], {});
88+
if (output.stdout.toLowerCase().includes('available')){
89+
return svc;
90+
}
91+
} catch (ex){
92+
traceError('Checking whether notebook is importable failed', ex);
93+
}
94+
}
8195
}
96+
return pythonExecutionFactory.createActivatedEnvironment({interpreter: this._interpreter});
8297
});
8398
}
8499
public interpreter() : Promise<PythonInterpreter | undefined> {

‎src/client/datascience/jupyter/kernels/kernelService.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { traceDecorators, traceError, traceInfo, traceVerbose, traceWarning } fr
1616
import { IFileSystem } from '../../../common/platform/types';
1717
import { IPythonExecutionFactory } from '../../../common/process/types';
1818
import { IInstaller, InstallerResponse, Product, ReadWrite } from '../../../common/types';
19+
import { sleep } from '../../../common/utils/async';
1920
import { noop } from '../../../common/utils/misc';
2021
import { IEnvironmentActivationService } from '../../../interpreter/activation/types';
2122
import { IInterpreterService, PythonInterpreter } from '../../../interpreter/contracts';
@@ -250,9 +251,18 @@ export class KernelService {
250251
return;
251252
}
252253

253-
const kernel = await this.findMatchingKernelSpec({ display_name: interpreter.displayName, name }, undefined, cancelToken);
254-
if (Cancellation.isCanceled(cancelToken)) {
255-
return;
254+
let kernel = await this.findMatchingKernelSpec({ display_name: interpreter.displayName, name }, undefined, cancelToken);
255+
for (let counter = 0; counter < 5; counter += 1){
256+
if (Cancellation.isCanceled(cancelToken)) {
257+
return;
258+
}
259+
if (kernel){
260+
break;
261+
}
262+
traceWarning('Waiting for 500ms for registered kernel to get detected');
263+
// Wait for jupyter server to get updated with the new kernel information.
264+
await sleep(500);
265+
kernel = await this.findMatchingKernelSpec({ display_name: interpreter.displayName, name }, undefined, cancelToken);
256266
}
257267
if (!kernel) {
258268
const error = `Kernel not created with the name ${name}, display_name ${interpreter.displayName}. Output is ${output.stdout}`;

‎src/test/datascience/jupyter/kernels/kernelService.unit.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ suite('Data Science - KernelService', () => {
275275
const kernelName = installArgs[3];
276276
assert.deepEqual(installArgs, ['install', '--user', '--name', kernelName, '--display-name', interpreter.displayName]);
277277
await assert.isRejected(promise, `Kernel not created with the name ${kernelName}, display_name ${interpreter.displayName}. Output is `);
278-
});
278+
}).timeout(5_000);
279279
test('Fail if installed kernel is not an instance of JupyterKernelSpec', async () => {
280280
when(execService.execModule('ipykernel', anything(), anything())).thenResolve({ stdout: '' });
281281
when(installer.isInstalled(Product.ipykernel, interpreter)).thenResolve(true);

0 commit comments

Comments
 (0)