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

Add Support for Message Based Interrupt #741

Merged
merged 1 commit into from
Aug 13, 2021
Merged

Add Support for Message Based Interrupt #741

merged 1 commit into from
Aug 13, 2021

Conversation

afshin
Copy link
Member

@afshin afshin commented Aug 13, 2021

Implements handling of interrupt_request message, which is now possible due to the control channel being on a separate thread.
Does not implement handling on Windows, which is less likely to be used for remote kernels.

@blink1073 blink1073 added this to the 6.2 milestone Aug 13, 2021
Copy link
Contributor

@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.

Excellent, thank you!

@blink1073
Copy link
Contributor

cc @SylvainCorlay @kevin-bates

@blink1073 blink1073 merged commit 05aa0e8 into ipython:master Aug 13, 2021
@SylvainCorlay
Copy link
Member

Thanks for this!
Happy to see ipykernel move closer to fully implementing the protocol ;)

@ccordoba12
Copy link
Member

I think this is not enough because it's missing a call to perform the request in jupyter_client, just like there's one for shutdown:

https://github.com/jupyter/jupyter_client/blob/9c0b4c0e28a9c17d9c14123522415b2048bd9522/jupyter_client/client.py#L762-L781

Otherwise, how is it possible to perform an interrupt from the kernel client?

@kevin-bates
Copy link

Hi @ccordoba12 - the interrupt machinery comes from the KernelManager by virtue of the kernelspec indicating support for message-based interrupts.

Hmm - should we make signal-based interrupts the else so that kernelspecs that don't specify a mode attempt at least that form of interrupt?

@blink1073
Copy link
Contributor

blink1073 commented Aug 16, 2021

According to the docs, it is opt-in: "For this to work, the kernels kernelspec must set interrupt_mode to message". I think the question is whether the ipykernel default spec should be message based.

@kevin-bates
Copy link

kevin-bates commented Aug 16, 2021

ok - I understood the question to be one of missing logic - sorry about that.

My question was more general and I just found the answer in that 'signal' will be used if interrupt_mode is not specified, which, now that I recall, have seen before.

I'll let the maintainers decide what the default mode should be. 😄

@ccordoba12
Copy link
Member

the interrupt machinery comes from the KernelManager by virtue of the kernelspec indicating support for message-based interrupts.

Yeah, but if you connect to a kernel started elsewhere (for example, locally to a kernel in a server, which Spyder users do a lot), then you don't have access to a kernel manager, just a kernel client. And in that case it'd be really nice to be able to send an interrupt message using the control channel (like you can now to perform a shutdown with the method I referenced above).

I thought this PR was about that, but it seems I was mistaken, sorry.

@kevin-bates
Copy link

I'm sorry, I probably steered this discussion in the wrong direction and I apologize. Please continue.

@ccordoba12
Copy link
Member

No worries @kevin-bates. Let's ping @davidbrochart to see what he thinks about this one.

@davidbrochart, do you think it's worth it to add a message-based interrupt in jupyter-client?

@SylvainCorlay
Copy link
Member

I think it is worth it just for the sake of completeness.

@ccordoba12
Copy link
Member

Agreed. I'll open a PR in jupyter-client so that (hopefully) it can be part of its 7.0 version.

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

Successfully merging this pull request may close these issues.

5 participants