-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
use pyzmq tools where appropriate #1030
Conversation
ZMQStream is the right object to use for event-driven handling of messages, but instead we chose to rewrite half of it in KernelManager. This removes most of the duplicate code, in favor of using ZMQStream. also use the pyzmq install() function for using pyzmq with tornado, instead of manually pasting its contents in notebook app.
Mmh, with this I can't start the notebook at all. Does it mean it changes the minimum tornado version required?
|
Sorry, the The parallel code on Windows already depends on fixes in pyzmq-2.1.7, so we could consider updating the baseline. |
I'd suggest not forcing unix users to update to > 2.1.4 if we don't really need it (in my case I can do it easily, but someone could be in a situation where they have for some reason a harder time), but emitting a warning if you have to use the fallback. That way we prod people to upgrade to avoid the warning. What do you think of that approach? |
Since it's just this one function that we need, which is literally two lines of code, I'd leave out the warning. There are very few changes of any significance to people not using pyzmq directly, after 2.1.4. Most of the changes to pyzmq since then are user-level sugar, or related to libzmq-3.x compatibility. |
Got it. In that case, merging now. I tested it and everything seemed OK, the code looks clean, and I'm glad to see this kind of cleanup. Thanks! |
Use pyzmq tools when available instead of duplicating functionality. ZMQStream is the right object to use for event-driven handling of messages, but instead we had a duplication of half of it in KernelManager. Also use the pyzmq install() function for using the pyzmq eventloop with tornado, instead of copying its contents into notebookapp.
Use pyzmq tools when available instead of duplicating functionality. ZMQStream is the right object to use for event-driven handling of messages, but instead we had a duplication of half of it in KernelManager. Also use the pyzmq install() function for using the pyzmq eventloop with tornado, instead of copying its contents into notebookapp.
ZMQStream is the right object to use for event-driven handling of messages, but instead we had a duplication of half of it in KernelManager.
This removes most of the duplicate code, in favor of using ZMQStream.
also use the pyzmq install() function for using the pyzmq eventloop with tornado, instead of copying its contents into notebookapp.