-
Notifications
You must be signed in to change notification settings - Fork 286
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
New KernelLauncher API for kernel discovery system #308
Conversation
MetaKernelFinder -> KernelFinder
Prototype new kernel discovery machinery
Fix typo in documentation.
The old URL points to a "This page has moved"-page
Updated URL for Jupyter Kernels in other languages
- use IOLoop.current over IOLoop.instance - drop removed `loop` arg from PeriodicCallback - deprecate now-unused IOLoopKernelRestarter.loop
- interrupt_mode="signal" is the default and current behaviour - With interrupt_mode="message", instead of a signal, a `interrupt_request` message on the control port will be sent
prepare for tornado 5
Additional to the actual signal, send a message on the control port
The test failure on Python 3.3 is due to a problem with pytest: pytest-dev/pytest#2966 The latest pytest release dropped support for Python 3.3, but a packaging problem as yet unknown means that that is not showing up in the metadata, so pip tries and fails to install the latest version. As this branch is intended to be for jupyter_client 6.0, I'm inclined to drop the 3.3 tests, but we may need to work around it for 5.x if pytest doesn't fix it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I like the simple launcher API.
So I plan to make KernelManager work with owned kernels (where it has a KernelLauncher) and non-owned kernels (where it does not).
I'm not sure about this. I think KernelManager should only work with owned Kernels. Everything that works with remote Kernels should be part of KernelClient. So rather than having Manager, Launcher, and Client, we should have just two: Manager + Client in early forms, or Launcher + Client in this new API. The primary motivation for the original KernelNanny proposal was for KernelClient to get all functionality for dealing with a Kernel, regardless of remote or local (interrupt, restart being the main missing pieces), and KernelManager would only be the implementation of managing a local process. This new Launcher API could take us there but it seems to me like it should mean dropping KernelManager entirely, rather than adding a third API. What do you think?
Or do you think there's enough logic that belongs in KernelManager and not in Client that Manager should get these extra abstractions around Launchers and stick around?
Async
👍 I'd love lots of test coverage for the new APIs.
Code duplication
I think code duplication is a good route to go for an upgrade to a new API. It allows us to clearly isolate and improve what the new implementation does without fear of breaking what the older implementations did. And gives us a clearer path to deprecation and eventual removal of the older APIs.
jupyter_client/async_launcher.py
Outdated
def wait(self): | ||
"""Wait for the kernel process to exit. | ||
""" | ||
raise NotImplementedError() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is only this method NotImplemented, while the others pass
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A spec for the return value of wait would be useful.
""" | ||
buf = os.urandom(16) | ||
return u'-'.join(b2a_hex(x).decode('ascii') for x in ( | ||
buf[:4], buf[4:] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4 + 4 = 8, not 16. Typo?
jupyter_client/async_launcher.py
Outdated
return (yield from self.in_default_executor(self.wrapped.launch)) | ||
|
||
@asyncio.coroutine | ||
def wait(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure we inherit docstrings
Also +1 to dropping Python 3.3 in 6.0 |
It does make sense for KernelManager to be for only an owned kernel. I'll have a look at moving some pieces from KernelManager to KernelClient. Maybe that will be enough to allow unifying kernel launchers with kernel managers. |
Thanks Yuvi. To clarify, would that be one kernel per container(/pod), so |
One kernel per container/pod, so containers/pods will be as ephemeral as
kernels.
…On Mon, Feb 12, 2018 at 6:23 AM, Thomas Kluyver ***@***.***> wrote:
Thanks Yuvi. To clarify, would that be one kernel per container(/pod), so docker
run ... or an equivalent command would start the kernel directly? Or
would the container be a longer-lasting thing that can start and stop
multiple kernels inside itself?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#308 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAB23kPxJ2eRa0IOH8OX-RTOvg3QMQW_ks5tUEl-gaJpZM4Qwr4o>
.
--
Yuvi Panda T
http://yuvi.in/blog
|
I should probably watching this PR with more regularity and contribute / review where I can. 😄 |
@yuvipanda I've made a rough prototype kernel provider to start a docker container locally and connect to it: https://github.com/takluyver/jupyter_docker_kernels This is very much a prototype, and it uses docker directly rather than any of the higher level management tools, but hopefully it gives you some idea of what it would take to use this API for docker. |
I'm starting to wonder whether, rather than having KernelManager2, KernelClient2, JupyterConsoleApp2 inside |
I converted https://github.com/Cadair/jupyter_environment_kernels to use the new infrastructure: Cadair/jupyter_environment_kernels#35 I've implemented it with two providers: one for conda, one for virtualenv. conda actually searches for python and IRkernel kernels. Here are some observations:
|
env.pop('PYTHONEXECUTABLE', None) | ||
|
||
if extra_env: | ||
print(extra_env) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug print...
Sorry for the timing (and verbosity)... tl;dr We're doing similar things in order to support remote kernels and very interested in this effort. Regarding the use case for remote kernels, this is almost entirely what Jupyter Enterprise Gateway provides - remote kernel management. Kernels used in data sciences tend to consume large amounts of resources. By supporting remote kernels (accessed via remote notebooks using NB2KG) we're able to better leverage cluster resources by spreading the kernels across the cluster. Since we cater to Spark-based analytics, we currently support the YARN resource manager in both client and cluster mode. In cluster mode, we let YARN determine where the kernel is going to land within the cluster. We also have an ssh-based "distributed" implementation. Because we want to avoid having to modify kernels, we wrap kernels in language-specific kernel launchers (yes, overloaded). I believe these provide similar functionality to the Nanny functionality. We also create a 6th "communication" port that is used for invoking interrupts and another means of conveying shutdown actions, etc. The message-based interrupts give us most of this functionality, but not all kernels support those. Each of the types of Resource Managers can be plugged into the framework via the notion of process proxies - which, I believe, are akin to the If you look into our repo, you'll notice we derive from As a result, we are very interested in this work. |
@takluyver What is the roadmap for this and https://github.com/takluyver/jupyter_kernel_mgmt + https://github.com/takluyver/jupyter_protocol ? Will jupyer_client go away? Also: how do I get jupyter_kernel_mgmt into a normal notebook server? |
My thinking is that
It will need changes to the notebook server code. And there's an extra bit that I haven't really worked out for the notebook server: how to pick a kernel to start when opening an existing notebook. I think this is at the heart of why so many people get confused by our current system. I'm planning to start trying to integrate this system into |
@takluyver Could you specify what "won't go beyond version 5.x releases" means? I'm trying to decide if it makes sense to already integrate this into jupyter_environment_kernels nowish or if I should wait (e.g. until it is testable in a jupyter notebook -> we have a use case that we want to update the list during runtime as we want new environments to be picked up during the lifetime of a notebook server). |
I mean that, if we follow this plan, there will probably never be a I'd be keen for you to try updating |
Ok, will try to get that done and only do tests with the |
Thanks! I'm working now on integrating it with a branch of nbconvert for a more interesting test case. Once that's working, I'll also get those packages on PyPI so that it's a bit easier to test with them. Feel free to ask about the new APIs; it's all still rather messy, and I haven't written much about it. The readmes of j_protocol and j_kernel_mgmt have a few details. |
@jankatins I've now got a branch of nbconvert working with the It needs an up to date version of jupyter_kernel_mgmt from my Github repo, because I've been discovering and fixing problems in that code as I worked on nbconvert. There were some hard to debug async issues to work out. |
And I've just put jupyter_protocol and jupyter_kernel_mgmt on PyPI, both at version 0.1 to emphasise that they're not yet stable. |
@takluyver if there are hard-to-debug async issues… would it be easier if we were to use stdlib asyncio and async/await and make the new kernel mechanisms ( From what I understood On top of that… the biggest reason I could imagine wanting to keep it python2 compatible is if we wanted to move all our python2 dependencies to use the new kernel management system and deprecate the jupyter_client system. Maybe I'm being pessimistic, but it seems unlikely that we're going to be able to completely drop |
Yup, I agree, and I'm actually already using asyncio. The two new packages currently require Python 3.4 or above. In fact the code is a bit of an ugly mixture of asyncio parts and tornado parts, since they now run on the same event loop. pyzmq has more convenient integration (ZMQStream) with tornado's API than it does with asyncio's. The main problem that I eventually figured out was the classic ZMQ slow subscriber issue, where you miss some messages on a PUB-SUB socket because they're sent before the subscription updates. We were declaring the client 'ready' when it got a kernel info reply, but that doesn't necessarily mean the iopub socket is getting output. So I now made it repeatedly send kernel_info_requests until it gets something (probably a status message) on iopub. That does rely on the kernel sending status messages. |
b1fe8e4
to
3e8ee4a
Compare
@takluyver I presume these kernel_discovery PRs to jupyter_client ought to be closed now that this work is being doine in kernel_mngt and jupyter_protocol? |
Thanks again for pushing on this @takluyver. Closing in favor of the Kernel Provisioning and Parameterized Kernel Launch work. |
@minrk I'd like to get your thoughts on the design of this before I get into integrating this machinery with KernelManager. It's meant to address #301.
The design we worked out in #261 remains: kernel providers are classes, discovered by entry points, which can tell Jupyter about kernel types from different systems (e.g. kernelspecs, conda environments, remote machines...).
The
make_manager()
method defined in #261 is gone, replaced bylaunch()
andlaunch_async()
. These return kernel launcher objects (better names welcome), which offer a subset ofPopen
methods, plusget_connection_info()
, which returns a dictionary of connection info (the same info you get from a connection file).Why a new interface? I wanted to use KernelManager, and split off the subclasses
QtKernelManager
andIOLoopKernelManager
as separate functionality outside of the manager. But KernelManager has grown all kinds of complexity, like sending messages over the control channel, which you can do even if you didn't start the kernel. So I plan to make KernelManager work with owned kernels (where it has a KernelLauncher) and non-owned kernels (where it does not).Async: so far, it has been OK for kernel control to be mostly synchronous. With increasing flexibility in how kernels are launched, this may be more painful. But I don't want to make N kernel providers support M event loops. So, asyncio. Tornado is moving in that direction, there's an asyncio interface for Qt event loops (quamash), and we're planning for our applications to require Python 3 in the next couple of years. I have also implemented an async wrapper, which runs the blocking kernel manager in a separate thread, so kernel providers only need to implement the blocking launcher interface - but there may be efficiency/reliability benefits to implementing an async interface rather than wrapping a blocking interface.
Where next?
KernelManager
use the (blocking) PopenKernelLauncher to launch kernels for code using KernelManager to start kernels. Allow passing a KernelLauncher into a KernelManager, for code using the discovery mechanism to start kernels.Code duplication: The new
launcher2
module contains quite a bit of duplicated code for launching kernels in a subprocess. The steps for launching a kernel are currently split between themanager
,connect
andlauncher
modules, and pulling them altogether was the only way to get a clear view of what's actually happening. I hope to eliminate the duplication again, but it's non-trivial because the code is written with a lot more flexibility than it probably needs.Version 6: These changes will become jupyter_client 6.0.