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

Fix packages of exceptions catched by gateway manager #5055

Merged

Conversation

shuichiro-makigaki
Copy link
Contributor

gateway_request in gateway.manager raises tornado.web.HTTPError exceptions, but the callers, such as GatewayKernelManager.get_kernel, catch tornado.httpclient.HTTPError, instead of tornado.web.HTTPError. Therefore, the callers can not handle exceptions during gateway interactions.

This causes that, for example, when Jupyter Enterprise Gateway culled a kernel by idle timeout, the gateway manager can not handle the kernel's absent appropriately. As a result, notebook users see ambiguous "Kernel Error" and can not restart the kernel or start a new kernel.

@shuichiro-makigaki shuichiro-makigaki changed the title Fix packages of exceptions catched by gateway manager [WIP] Fix packages of exceptions catched by gateway manager Nov 15, 2019
@shuichiro-makigaki shuichiro-makigaki force-pushed the fix-gateway-handled-exceptions branch from 958e702 to ee28120 Compare November 15, 2019 09:40
@shuichiro-makigaki shuichiro-makigaki changed the title [WIP] Fix packages of exceptions catched by gateway manager Fix packages of exceptions catched by gateway manager Nov 15, 2019
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

@shuichiro-makigaki - thank you for these changes! I wonder if something was introduced that prevents restarts or new starts following culling because this should have been seen before. I've confirmed these changes - thanks again!

@kevin-bates
Copy link
Member

@shuichiro-makigaki - would you mind submitting essentially this same PR against jupyter_server?

`gateway_request` in `gateway.manager` raises `tornado.web.HTTPError` exceptions,
but the callers, such as `GatewayKernelManager.get_kernel`, catch
`tornado.httpclient.HTTPError`, instead of `tornado.web.HTTPError`.
Therefore, the callers can not handle exceptions during gateway interactions.

This causes that, for example, when Jupyter Enterprise Gateway culled a kernel
by idle timeout, the gateway manager can not handle the kernel's absent appropriately.
As a result, notebook users see ambiguous "Kernel Error" and can not restart
the kernel or start a new kernel.
@shuichiro-makigaki shuichiro-makigaki force-pushed the fix-gateway-handled-exceptions branch from ee28120 to aa73eb4 Compare November 16, 2019 09:18
@shuichiro-makigaki
Copy link
Contributor Author

shuichiro-makigaki commented Nov 16, 2019

I added some fixes to tests also.

would you mind submitting essentially this same PR against jupyter_server?

Sure! jupyter-server/jupyter_server#139

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

LGTM

@kevin-bates kevin-bates added this to the 6.0.3 milestone Nov 23, 2019
@takluyver takluyver merged commit 7b5248d into jupyter:master Nov 27, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 26, 2021
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.

3 participants