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

Reconnect to kernel on restart action #9371

Merged
merged 1 commit into from Nov 20, 2020

Conversation

kevin-bates
Copy link
Contributor

@kevin-bates kevin-bates commented Nov 19, 2020

This pull request addresses the issue raised in #8103 and closed by #8432. That change was not sufficient as the reconnect() was embedded within the _handleRestart() method - which is conditionally called as a result of execution states of autorestarting or restarting, only the latter of which is returned by the server and only when the kernel is automatically restarted. No execution status is returned from the server upon completion of a manual restart request. As a result, the fixed code will never be reached upon a manual restart.

Please note that relative to the originating issue, neither JupyterHub nor (arguably) Enterprise Gateway are related. The issue is really about whether a kernel's ZMQ ports are refreshed on manual restarts. For local kernels, that is atypical. For remote kernels deployed across managed clusters, it is the norm since the kernel's physical location is likely to change. EG happens to be a way kernels are deployed across a cluster.

This issue does not occur when using the Notebook UI - whose kernel restart behavior includes the reconnect sequence. As a result, this change brings parity with Notebook.

References

JupyterHub+JupyterLab+JupyterEnterpriseGateway Restart Kernel hangs #8013

Reconnect a websocket when a kernel is restarted. #8432

Code changes

We now explicitly call reconnect() immediately following the conclusion of the kernel restart request. This change was recommended by @blink1073 in #8013 (comment).

User-facing changes

Restarts of kernels, irrespective of location, now yield a working (connected) kernel.

Backwards-incompatible changes

None - this change introduces parity with the Notebook UI wrt kernel restarts.

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link: Binder

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 added this to the 3.0 milestone Nov 20, 2020
@blink1073 blink1073 merged commit 7209615 into jupyterlab:master Nov 20, 2020
@blink1073
Copy link
Member

Congratulations @kevin-bates on your first PR to JupyterLab! 🎉

@kevin-bates kevin-bates deleted the reconnect-on-restart branch November 20, 2020 17:05
@kevin-bates kevin-bates restored the reconnect-on-restart branch November 20, 2020 23:12
@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label May 20, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:services status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants