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

Support kernel_info request on the control channel #82

Merged
merged 6 commits into from
Dec 11, 2023

Conversation

JohanMabille
Copy link
Member

@JohanMabille JohanMabille commented Dec 3, 2021

In this JEP we propose the kernel_info request can be sent on both the shell and the control channels, to fix issues like this and this.

cc @SylvainCorlay and @minrk who I have talked to about this JEP.

Voting from @jupyter/software-steering-council

@ivanov
Copy link
Member

ivanov commented Dec 17, 2021

From the description of this proposal, it seems like the problem at hand can be solved at the Jupyter Server level, without introducing backwards incompatible protocol changes. Can jupyter server be changed to issue its initial kernel_info_request on the control channel, and also not assume it has to receive a reply to that message before deeming a connection as established?

It makes sense to encourage clients to issue kernel_info requests over control channel, but dropping support for it on the shell channel would be a major protocol version change.

@JohanMabille
Copy link
Member Author

@ivanov I agree that a backward incompatible change is not ideal. We could do what we did with kernel_interrupt and kernel_shutdown requests: we move the description of the message to the Messages on the control channel section of the protocol, and mark it as deprecated on the shell channel. We would have nothing to change in the implementation of Jupyter Server for now, since it sends kernel_info on both channels thanks to the last fix by @Zsailer. If you agree with that, I will update the JEP accordingly.

@ivanov
Copy link
Member

ivanov commented Dec 23, 2021

@JohanMabille I'm still not sure that we want to necessarily deprecate the message pair on the shell channel. I understand that it's easier for implementors if a given message should only appear on one channel. However, one of the affordances with the shell channel approach is the current pattern of clients issuing a kernel_info_request on connect, and indicating that the connection is not complete and that the kernel is busy or otherwise not yet known to be available until it gets back a reply. So it serves as a queued no-op with respect to kernel state, and does not require the client to know the language of the kernel.

Specific example would be similar to the debugging example in the JEP but suppose that instead of debugging, you have a long running computation (like sleep 100) that is running when a new client connects. If the new client makes a kernel info request on the control channel, and the kernel handles that request, the client still does now know whether the kernel is in busy or idle state until it makes an execute request. And even after making an execute request, client has no way of indicating if the pending execution is for the new code, or for the completion of previously running code, followed by the new code.

One possibility could be to add a kernel status message to the body of a kernel_info_reply content, to resolve this ambiguity for the client in the absence of kernel status messages.

@SylvainCorlay
Copy link
Member

On the core of this proposal, we clearly need to require kernel info to be supported on the control channel, for situations such as connecting to a kernel that is currently blocked on a breakpoint or currently executing a long-running cell. In my opinion, this should be resolved at the protocol level because it also applies to cases where there isn't a server (connecting to an existing kernel).

On the other point brought up by @ivanov on the semantics of kernel idle and busy messages: We would like to make some proposals at a later stage on this subject. My take on this is that idle and busy messages should eventually only concern the state of the Shell channel (it is not of interest to kernel end-users to know whether the control channel is processing messages. Control channel messages ought to be processed fast in a non-blocking way as much as possible).
Kernels such as akernel which can process several execution requests concurrently also challenge the current model.

@minrk
Copy link
Member

minrk commented May 4, 2023

Updating comments on this, I agree with requiring support for kernel_info on control, and agree with not deprecating it at all on shell. That doesn't appear to have a material benefit, and does have a breaking cost.

@JohanMabille JohanMabille changed the title Move kernel_info request on the control channel proposal Support kernel_info request on the control channel Nov 13, 2023
@JohanMabille
Copy link
Member Author

I rewrote the proposal to state that the kernel_info request should be supported on both Shell and Control.

Co-authored-by: gabalafou <gabriel@fouasnon.com>
Co-authored-by: gabalafou <gabriel@fouasnon.com>
@Zsailer
Copy link
Member

Zsailer commented Dec 4, 2023

I'm in favor of this PR. This is already implemented in Jupyter Server (and Jupyverse).

I proposed some minor changes to the JEP description to include a few extra details that might useful for future reference.

JohanMabille and others added 2 commits December 4, 2023 20:42
Co-authored-by: Zachary Sailer <zachsailer@gmail.com>
Co-authored-by: Zachary Sailer <zachsailer@gmail.com>
@JohanMabille
Copy link
Member Author

7 yes, 1 abstention, and 2 non votes, let's merge!

@JohanMabille JohanMabille merged commit 6d9a65f into jupyter:master Dec 11, 2023
1 check passed
@JohanMabille JohanMabille deleted the kernel_info branch December 11, 2023 16:20
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.

7 participants