-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
add KernelManager.exit_status #958
base: main
Are you sure you want to change the base?
Conversation
A 0 value seems reasonable at this time.
Kernel implementations typically handle |
@tmaxwell-anthropic any opinions on the detailed behaviour, or in what ways it would be maximally useful for cases such as yours? See in-line comments. Fixed CI issues*, and testing to see how signals that are defined on windows behave. * why is mypy not configured to be run in pre-commit? And |
Thanks for looking into this! I don't know much about Jupyter internals, but here are some thoughts:
|
The auto-restarter is stopped for any flavor of intentional "shutdown" (either directly or on behalf of intentional restarts). It lives only to detect unexpected terminations. |
Adding a parameter with the exit code to the callback would either break all existing code, or require inspecting the callable to see if it can take a parameter with the signature or not. The latter is definitely doable, but not sure if it's the best way go to about it. |
Another option might be to add a second set of callbacks, with different names, which are equivalent except that they take an extra parameter. So the component that's registering the callback could indicate whether it wants a parameter based on which callback name it registers for. |
Ah that's pretty nice. Or probably even cleaner, add a parameter |
…arter pass the exit code to callback when restarting
…mplementation instead
made a somewhat messy (esp when you look at the typing) implementation for restarter callback taking the exit status, and wrote (copy-paste+modified) a test for it. A more creative test would kill the process in several different ways, but that should be covered by |
Quick draft implementation of #957 in the way suggested by @kevin-bates
if not self.has_kernel
, both0
andNone
seem plausible - and could also go with a positive value.I tested all possible signals, and a bunch of them were ignored - including
signal.SIGINT
. Not sure if that's intended.