Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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