Skip to content

WIP: Use new kernel management APIs in notebook server #4170

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

Closed
wants to merge 50 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
0dc8b70
Initial work towards using jupyter_kernel_mgmt for notebook server
takluyver Oct 25, 2018
ae15888
Provide kernel specs from kernel finder machinery
takluyver Oct 26, 2018
ddcc3dd
Display kernel type ID alongside display name
takluyver Oct 26, 2018
27fc52a
Add dependency on jupyter_kernel_mgmt & jupyter_protocol
takluyver Oct 26, 2018
102ae7b
Switch single kernelspec API handler to kernel_finder
takluyver Oct 26, 2018
5765854
Drop kernel_spec_manager
takluyver Oct 26, 2018
1dfc5d4
Fix interrupting a kernel
takluyver Oct 30, 2018
76ab738
Fix up kernelspec API tests to pass
takluyver Oct 30, 2018
f1a79bf
Default kernel type name pyimport/kernel
takluyver Oct 31, 2018
b0e0fec
Use pyimport/kernel for session API tests
takluyver Oct 31, 2018
fd0b60c
Use pyimport/kernel for kernels API tests
takluyver Oct 31, 2018
c76e6bf
Fix dummy objects for SessionManager tests
takluyver Oct 31, 2018
2d9e021
Use reworked restarter callback system
takluyver Oct 31, 2018
a15a1f9
Kernel restart always means new manager & new connection info
takluyver Oct 31, 2018
b427507
Use updated API for message handlers
takluyver Oct 31, 2018
90f9e68
Restarter callbacks parameters swapped again
takluyver Oct 31, 2018
f6e7cfd
Clean up some unused imports
takluyver Oct 31, 2018
d87f862
Reworked kernel restart handling
takluyver Nov 1, 2018
24d3d0b
Reorganise code for kernel websocket handler
takluyver Nov 1, 2018
cdd45a6
Require jupyter_kernel_mgmt >=0.3
takluyver Nov 1, 2018
9ed09ae
Change selectors used by tests to create new notebook
takluyver Nov 1, 2018
00ab20c
Fix race condition making AJAX requests for session
takluyver Nov 1, 2018
20e9db4
Fix selector for new kernel menu entry
takluyver Nov 1, 2018
69a79ac
Try to get more information on test failure
takluyver Nov 1, 2018
d63cd96
Don't try to forward message from kernel when websocket closed
takluyver Nov 3, 2018
237a212
Avoid race condition creating duplicate sessions
takluyver Nov 3, 2018
80c278a
Fix starting a kernel when modifying a session
takluyver Nov 4, 2018
0a18fb9
Fix sessionmanager tests
takluyver Nov 4, 2018
c3d63d0
Propagate future out for sending status messages
takluyver Nov 5, 2018
65d49a4
Don't crash closing websocket if handler not connected
takluyver Nov 5, 2018
6efb932
Rework how kernel startup works again
takluyver Nov 6, 2018
8233daa
msg_type is part of the message header, not top-level
takluyver Nov 6, 2018
e80cafe
There is no longer a good way to prevent a kernel from restarting
takluyver Nov 6, 2018
cab3416
Wait to send starting status until new client is reconnected
takluyver Nov 6, 2018
a8dbd48
Expect kernel_starting event in autorestart test
takluyver Nov 6, 2018
f6b0674
Ensure message in frontend always has buffers array
takluyver Nov 6, 2018
76b9044
Wait for kernel to be connected to send shutdown_request
takluyver Nov 6, 2018
c42fb21
There is no longer a good way to prevent a kernel auto-restarting
takluyver Nov 6, 2018
336a321
404 error for websocket opened to nonexistant kernel
takluyver Nov 6, 2018
3ab2d12
Wait for kernel for cell mode-switching tests
takluyver Nov 6, 2018
826ee65
Message for callback is converted to update_display_data
takluyver Nov 6, 2018
bb54f34
Fix deserialisation test
takluyver Nov 6, 2018
3bc13fb
Allow for kernel state changing during session test
takluyver Nov 6, 2018
e4eff7a
Allow execution_state to change in test_modify_kernel_id
takluyver Nov 6, 2018
e5bb55c
Use appropriate attribute in cull debug
kevin-bates Jun 7, 2019
d721064
Culling should go through MappingKernelManager
kevin-bates Jun 7, 2019
c7b4042
Merge pull request #1 from kevin-bates/patch-1
takluyver Jun 13, 2019
9d3c7e0
Merge pull request #2 from kevin-bates/patch-2
takluyver Jun 13, 2019
ab84f2a
Use kernel_id from provider's manager
kevin-bates Jul 22, 2019
54fdb14
Merge pull request #3 from gateway-experiments/jupyter-kernel-mgmt
takluyver Jul 22, 2019
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
10 changes: 5 additions & 5 deletions notebook/base/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,11 @@ def contents_js_source(self):
#---------------------------------------------------------------
# Manager objects
#---------------------------------------------------------------


@property
def kernel_finder(self):
return self.settings['kernel_finder']

@property
def kernel_manager(self):
return self.settings['kernel_manager']
Expand All @@ -283,10 +287,6 @@ def session_manager(self):
@property
def terminal_manager(self):
return self.settings['terminal_manager']

@property
def kernel_spec_manager(self):
return self.settings['kernel_spec_manager']

@property
def config_manager(self):
Expand Down
172 changes: 1 addition & 171 deletions notebook/base/zmqhandlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,85 +4,12 @@
# Copyright (c) Jupyter Development Team.
# Distributed under the terms of the Modified BSD License.

import os
import json
import struct
import warnings
import sys

try:
from urllib.parse import urlparse # Py 3
except ImportError:
from urlparse import urlparse # Py 2

import tornado
from tornado import gen, ioloop, web
from tornado.websocket import WebSocketHandler

from jupyter_client.session import Session
from jupyter_client.jsonutil import date_default, extract_dates
from ipython_genutils.py3compat import cast_unicode

from .handlers import IPythonHandler

def serialize_binary_message(msg):
"""serialize a message as a binary blob

Header:

4 bytes: number of msg parts (nbufs) as 32b int
4 * nbufs bytes: offset for each buffer as integer as 32b int

Offsets are from the start of the buffer, including the header.

Returns
-------

The message serialized to bytes.

"""
# don't modify msg or buffer list in-place
msg = msg.copy()
buffers = list(msg.pop('buffers'))
if sys.version_info < (3, 4):
buffers = [x.tobytes() for x in buffers]
bmsg = json.dumps(msg, default=date_default).encode('utf8')
buffers.insert(0, bmsg)
nbufs = len(buffers)
offsets = [4 * (nbufs + 1)]
for buf in buffers[:-1]:
offsets.append(offsets[-1] + len(buf))
offsets_buf = struct.pack('!' + 'I' * (nbufs + 1), nbufs, *offsets)
buffers.insert(0, offsets_buf)
return b''.join(buffers)


def deserialize_binary_message(bmsg):
"""deserialize a message from a binary blog

Header:

4 bytes: number of msg parts (nbufs) as 32b int
4 * nbufs bytes: offset for each buffer as integer as 32b int

Offsets are from the start of the buffer, including the header.

Returns
-------

message dictionary
"""
nbufs = struct.unpack('!i', bmsg[:4])[0]
offsets = list(struct.unpack('!' + 'I' * nbufs, bmsg[4:4*(nbufs+1)]))
offsets.append(None)
bufs = []
for start, stop in zip(offsets[:-1], offsets[1:]):
bufs.append(bmsg[start:stop])
msg = json.loads(bufs[0].decode('utf8'))
msg['header'] = extract_dates(msg['header'])
msg['parent_header'] = extract_dates(msg['parent_header'])
msg['buffers'] = bufs[1:]
return msg
from tornado import ioloop

# ping interval for keeping websockets alive (30 seconds)
WS_PING_INTERVAL = 30000
Expand Down Expand Up @@ -192,100 +119,3 @@ def send_ping(self):
def on_pong(self, data):
self.last_pong = ioloop.IOLoop.current().time()


class ZMQStreamHandler(WebSocketMixin, WebSocketHandler):

if tornado.version_info < (4,1):
"""Backport send_error from tornado 4.1 to 4.0"""
def send_error(self, *args, **kwargs):
if self.stream is None:
super(WebSocketHandler, self).send_error(*args, **kwargs)
else:
# If we get an uncaught exception during the handshake,
# we have no choice but to abruptly close the connection.
# TODO: for uncaught exceptions after the handshake,
# we can close the connection more gracefully.
self.stream.close()


def _reserialize_reply(self, msg_or_list, channel=None):
"""Reserialize a reply message using JSON.

msg_or_list can be an already-deserialized msg dict or the zmq buffer list.
If it is the zmq list, it will be deserialized with self.session.

This takes the msg list from the ZMQ socket and serializes the result for the websocket.
This method should be used by self._on_zmq_reply to build messages that can
be sent back to the browser.

"""
if isinstance(msg_or_list, dict):
# already unpacked
msg = msg_or_list
else:
idents, msg_list = self.session.feed_identities(msg_or_list)
msg = self.session.deserialize(msg_list)
if channel:
msg['channel'] = channel
if msg['buffers']:
buf = serialize_binary_message(msg)
return buf
else:
smsg = json.dumps(msg, default=date_default)
return cast_unicode(smsg)

def _on_zmq_reply(self, stream, msg_list):
# Sometimes this gets triggered when the on_close method is scheduled in the
# eventloop but hasn't been called.
if self.stream.closed() or stream.closed():
self.log.warning("zmq message arrived on closed channel")
self.close()
return
channel = getattr(stream, 'channel', None)
try:
msg = self._reserialize_reply(msg_list, channel=channel)
except Exception:
self.log.critical("Malformed message: %r" % msg_list, exc_info=True)
else:
self.write_message(msg, binary=isinstance(msg, bytes))


class AuthenticatedZMQStreamHandler(ZMQStreamHandler, IPythonHandler):

def set_default_headers(self):
"""Undo the set_default_headers in IPythonHandler

which doesn't make sense for websockets
"""
pass

def pre_get(self):
"""Run before finishing the GET request

Extend this method to add logic that should fire before
the websocket finishes completing.
"""
# authenticate the request before opening the websocket
if self.get_current_user() is None:
self.log.warning("Couldn't authenticate WebSocket connection")
raise web.HTTPError(403)

if self.get_argument('session_id', False):
self.session.session = cast_unicode(self.get_argument('session_id'))
else:
self.log.warning("No session ID specified")

@gen.coroutine
def get(self, *args, **kwargs):
# pre_get can be a coroutine in subclasses
# assign and yield in two step to avoid tornado 3 issues
res = self.pre_get()
yield gen.maybe_future(res)
super(AuthenticatedZMQStreamHandler, self).get(*args, **kwargs)

def initialize(self):
self.log.debug("Initializing websocket connection %s", self.request.path)
self.session = Session(config=self.config)

def get_compression_options(self):
return self.settings.get('websocket_compression_options', None)
21 changes: 13 additions & 8 deletions notebook/kernelspecs/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,23 @@ def initialize(self):

@web.authenticated
def get(self, kernel_name, path, include_body=True):
ksm = self.kernel_spec_manager
try:
self.root = ksm.get_kernel_spec(kernel_name).resource_dir
except KeyError:
raise web.HTTPError(404, u'Kernel spec %s not found' % kernel_name)
self.log.debug("Serving kernel resource from: %s", self.root)
return web.StaticFileHandler.get(self, path, include_body=include_body)
kf = self.kernel_finder
# TODO: Do we actually want all kernel type names to be case-insensitive?
kernel_name = kernel_name.lower()
for name, info in kf.find_kernels():
if name == kernel_name:
self.root = info['resource_dir']
self.log.debug("Serving kernel resource from: %s", self.root)
return web.StaticFileHandler.get(self, path,
include_body=include_body)

raise web.HTTPError(404, u'Kernel spec %s not found' % kernel_name)


@web.authenticated
def head(self, kernel_name, path):
return self.get(kernel_name, path, include_body=False)

default_handlers = [
(r"/kernelspecs/%s/(?P<path>.*)" % kernel_name_regex, KernelSpecResourceHandler),
]
]
42 changes: 17 additions & 25 deletions notebook/notebookapp.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@
)
from jupyter_core.paths import jupyter_config_path
from jupyter_client import KernelManager
from jupyter_client.kernelspec import KernelSpecManager, NoSuchKernel, NATIVE_KERNEL_NAME
from jupyter_client.session import Session
from jupyter_kernel_mgmt.discovery import KernelFinder
from nbformat.sign import NotebookNotary
from traitlets import (
Any, Dict, Unicode, Integer, List, Bool, Bytes, Instance,
Expand Down Expand Up @@ -146,22 +146,22 @@ def load_handlers(name):
class NotebookWebApplication(web.Application):

def __init__(self, jupyter_app, kernel_manager, contents_manager,
session_manager, kernel_spec_manager,
session_manager, kernel_finder,
config_manager, extra_services, log,
base_url, default_url, settings_overrides, jinja_env_options):


settings = self.init_settings(
jupyter_app, kernel_manager, contents_manager,
session_manager, kernel_spec_manager, config_manager,
session_manager, kernel_finder, config_manager,
extra_services, log, base_url,
default_url, settings_overrides, jinja_env_options)
handlers = self.init_handlers(settings)

super(NotebookWebApplication, self).__init__(handlers, **settings)

def init_settings(self, jupyter_app, kernel_manager, contents_manager,
session_manager, kernel_spec_manager,
session_manager, kernel_finder,
config_manager, extra_services,
log, base_url, default_url, settings_overrides,
jinja_env_options=None):
Expand Down Expand Up @@ -252,10 +252,10 @@ def init_settings(self, jupyter_app, kernel_manager, contents_manager,
local_hostnames=jupyter_app.local_hostnames,

# managers
kernel_finder=kernel_finder,
kernel_manager=kernel_manager,
contents_manager=contents_manager,
session_manager=session_manager,
kernel_spec_manager=kernel_spec_manager,
config_manager=config_manager,

# handlers
Expand Down Expand Up @@ -566,7 +566,6 @@ class NotebookApp(JupyterApp):
classes = [
KernelManager, Session, MappingKernelManager,
ContentsManager, FileContentsManager, NotebookNotary,
KernelSpecManager,
]
flags = Dict(flags)
aliases = Dict(aliases)
Expand Down Expand Up @@ -1114,6 +1113,12 @@ def _update_mathjax_config(self, change):
(shutdown the notebook server)."""
)

kernel_providers = List(config=True,
help=_('A list of kernel provider instances. '
'If not specified, all installed kernel providers are found '
'using entry points.')
)

contents_manager_class = Type(
default_value=LargeFileManager,
klass=ContentsManager,
Expand All @@ -1139,20 +1144,6 @@ def _update_mathjax_config(self, change):
help=_('The config manager class to use')
)

kernel_spec_manager = Instance(KernelSpecManager, allow_none=True)

kernel_spec_manager_class = Type(
default_value=KernelSpecManager,
config=True,
help="""
The kernel spec manager class to use. Should be a subclass
of `jupyter_client.kernelspec.KernelSpecManager`.

The Api of KernelSpecManager is provisional and might change
without warning between this version of Jupyter and the next stable one.
"""
)

login_handler_class = Type(
default_value=LoginHandler,
klass=web.RequestHandler,
Expand Down Expand Up @@ -1308,14 +1299,15 @@ def parse_command_line(self, argv=None):
self.update_config(c)

def init_configurables(self):
self.kernel_spec_manager = self.kernel_spec_manager_class(
parent=self,
)
if self.kernel_providers:
self.kernel_finder = KernelFinder(self.kernel_providers)
else:
self.kernel_finder = KernelFinder.from_entrypoints()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a little awkward to me. How would an application be able to have providers from both the property and the entrypoints? If there was a way to get a list of entrypoint-based providers, then you could require the property be the composite.

If there was a separate way to get the providers from entrypoints, then you could just call the constructor and, if the argument is None, have it call from_entrypoints(), else the argument is the complete set.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was essentially just for testing, though I was planning to add some sort of inclusion/exclusion lists for real configurability. The idea is that it will always normally discover kernel providers from entry points, but in certain odd situations (like testing) you might want to override it so that you can completely control kernel discovery.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I would recommend these approaches get pushed into the constructor and let it determine how to compose/filter the providers.

self.kernel_manager = self.kernel_manager_class(
parent=self,
log=self.log,
connection_dir=self.runtime_dir,
kernel_spec_manager=self.kernel_spec_manager,
kernel_finder=self.kernel_finder,
)
self.contents_manager = self.contents_manager_class(
parent=self,
Expand Down Expand Up @@ -1373,7 +1365,7 @@ def init_webapp(self):

self.web_app = NotebookWebApplication(
self, self.kernel_manager, self.contents_manager,
self.session_manager, self.kernel_spec_manager,
self.session_manager, self.kernel_finder,
self.config_manager, self.extra_services,
self.log, self.base_url, self.default_url, self.tornado_settings,
self.jinja_environment_options,
Expand Down
Loading