-
Notifications
You must be signed in to change notification settings - Fork 283
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 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.