Skip to content
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

add kernel_info_timeout traitlet for slow kernel start/restart #3665

Merged
merged 1 commit into from Jun 8, 2018

Conversation

Projects
None yet
3 participants
@mpacer
Copy link
Member

mpacer commented Jun 6, 2018

This allows you to configure how long the notebook waits for a kernel give its kernel_info_reply.

I looked into adding a test, but I think that this would require a good bit of machinery to test properly. I can do that if we want to, but it looks like we're not testing the functionality of most other traitlets directly.

Note: this is not kernel specific… but I don't think we have any way to do kernel specific configuration for the handler as of today.

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Jun 6, 2018

@mpacer mpacer changed the title add restart_reply_timeout configurable traitlet for slow kernels WIP: add restart_reply_timeout configurable traitlet for slow kernels Jun 6, 2018

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Jun 6, 2018

I changed this to WIP because I want to make it also work for the initial startup timeout (as it is in jupyter_console).

Now it does this.

@mpacer mpacer changed the title WIP: add restart_reply_timeout configurable traitlet for slow kernels add restart_reply_timeout configurable traitlet for slow kernels Jun 6, 2018

@mpacer mpacer force-pushed the mpacer:reply_timeout branch from fc5f8d6 to 56055fb Jun 6, 2018

add kernel_info_timeout traitlet to wait for slow kernel startups
This affects both the MappingKernelManager and the ZMQChannelsHandler(by extension). This allows one setting to apply to both startup andrestarting.
@@ -93,6 +93,22 @@ def _update_root_dir(self, proposal):
no frontends are connected.
"""
)

kernel_info_timeout = Float(60, config=True,

This comment has been minimized.

@rgbkrk

rgbkrk Jun 6, 2018

Member

Excellent!

@rgbkrk

rgbkrk approved these changes Jun 6, 2018

Copy link
Member

rgbkrk left a comment

Super fan of this since we've been dealing with slower spark-backed kernels at Netflix.

@mpacer mpacer force-pushed the mpacer:reply_timeout branch from 56055fb to c6dd032 Jun 6, 2018

@mpacer mpacer changed the title add restart_reply_timeout configurable traitlet for slow kernels add kernel_info_timeout configurable traitlet for slow kernel startup Jun 6, 2018

@mpacer mpacer changed the title add kernel_info_timeout configurable traitlet for slow kernel startup add kernel_info_timeout traitlet for slow kernel start/restart Jun 6, 2018

@mpacer

This comment has been minimized.

Copy link
Member Author

mpacer commented Jun 8, 2018

@Carreau @gnestor @takluyver @minrk Are there any issues with this going forward?

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jun 8, 2018

Great!

@minrk minrk merged commit 2c061a4 into jupyter:master Jun 8, 2018

4 checks passed

codecov/patch 100% of diff hit (target 0%)
Details
codecov/project 74.47% (+<.01%) compared to 459b92c
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mpacer mpacer added this to the 5.6 milestone Jun 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.