Skip to content

Commit

Permalink
Start jupyter notebooks even if notebook is not installed in currenv …
Browse files Browse the repository at this point in the history
…env (microsoft#8977)

* Start jupyter notebooks even if notebook is not installed in currenv env

* Increase timeout of test
  • Loading branch information
DonJayamanne authored Dec 10, 2019
1 parent b96e7a3 commit 7e70cb2
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 48 deletions.
7 changes: 5 additions & 2 deletions pythonFiles/datascience/daemon/daemon_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
41 changes: 24 additions & 17 deletions pythonFiles/datascience/getServerInfo.py
Original file line number Diff line number Diff line change
@@ -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))
67 changes: 48 additions & 19 deletions pythonFiles/datascience/jupyter_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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()
8 changes: 8 additions & 0 deletions pythonFiles/datascience/jupyter_nbInstalled.py
Original file line number Diff line number Diff line change
@@ -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")
27 changes: 21 additions & 6 deletions src/client/datascience/jupyter/jupyterCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -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> {
Expand Down
16 changes: 13 additions & 3 deletions src/client/datascience/jupyter/kernels/kernelService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 7e70cb2

Please sign in to comment.