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

Enable restart_kernel for async usage #4412

Merged
merged 1 commit into from
Feb 25, 2019

Conversation

kevin-bates
Copy link
Member

Converted MappingKernelManager.restart_kernel to a coroutine so that projects that take advantage of async kernel startup can also realize appropriate behavior relative to restarts. To take full advantage of async kernel startup (with restarts), this PR along with its "sibling PR" will be required.

Please note, however, that each of these PRs can be independently installed w/o affecting today's notebook applications (as far as I can determine via testing the possible combinations). That said, BOTH of these PRs will be required for usage by Enterprise Gateway - which incurs very long kernel startup times - or other projects that require concurrent kernel starts.

It would be ideal to have this PR back-ported to python 2.7-relative release since EG has an immediate need.

Converted `MappingKernelManager.restart_kernel` to a coroutine so that
projects that take advantage of async kernel startup can also realize
appropriate behavior relative to restarts.
@kevin-bates
Copy link
Member Author

@minrk - thank you for merging.

What is the best way to request a back-port to 5.7.x since it would be ideal if this could be used on Python 2.x (along with its sibling PR jupyter/jupyter_client#425)?

I'd be happy to submit a PR on the 5.7.x branch if that's all that is necessary?

cc: @takluyver

kevin-bates added a commit to kevin-bates/notebook that referenced this pull request Feb 25, 2019
This is an "add-on" commit to PR jupyter#4412 in which kernel startup was
addressed.
@minrk minrk added this to the 5.7.5 milestone Mar 5, 2019
minrk added a commit that referenced this pull request Mar 5, 2019
Converted `MappingKernelManager.restart_kernel` to a coroutine so that projects that take advantage of async kernel startup can also realize appropriate behavior relative to restarts.  To take full advantage of async kernel startup (with restarts), this PR along with its "[sibling PR](jupyter/jupyter_client#425)" will be required.

Please note, however, that each of these PRs can be independently installed w/o affecting today's notebook applications (as far as I can determine via testing the possible combinations).  That said, BOTH of these PRs will be required for usage by Enterprise Gateway - which incurs very long kernel startup times - or other projects that require concurrent kernel starts.

It would be ideal to have this PR back-ported to python 2.7-relative release since EG has an immediate need.

Signed-off-by: Min RK <benjaminrk@gmail.com>
@minrk minrk modified the milestones: 5.7.5, 6.0 Mar 5, 2019
@minrk
Copy link
Member

minrk commented Mar 5, 2019

I started backporting it to 5.7.x, but since it's a backward-incompatible API change, that wouldn't be appropriate, so I've marked it for 6.0.

@kevin-bates
Copy link
Member Author

This is really unfortunate. This essentially means that our gateway 1.x line can never support async kernel startup since those customers still require Python 2 which can't coexist with NB 6.

Do you know of applications that call restart_kernel directly besides Enterprise Gateway (which does so via inheritance)? I understand you can't know the complete answer, just curious if its non-zero.

@minrk
Copy link
Member

minrk commented Mar 5, 2019

Ah! I misread this PR. restart_kernel is already returning a Future, so we should be okay backporting this one. The jupyter-client PRs are still more complicated, but I think this one will be okay. I have to go, but I'll look into this one a bit more in the morning.

@minrk minrk modified the milestones: 6.0, 5.7.5 Mar 5, 2019
@kevin-bates
Copy link
Member Author

Thanks great news Min - thank you. Do you have a timeframe for the 6.0 releases for NB and JC? We'd like to coordinate our EG 2.0 GA with those if they're in the ballpark of end of March.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants