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
Make handling comm messages optional in a kernel connection. #6929
Conversation
The comm message protocol currently has implicit assumptions that only one kernel connection is handling comm messages. This option allows a kernel connection to opt out of handling comms. See jupyter/jupyter_client#263 This fixes the error where ipywidgets stops working if you open a notebook and a console to the same kernel, since the console automatically closes all comms since it does not have the comm target registered.
Thanks for making a pull request to JupyterLab! To try out this branch on binder, follow this link: |
Should we additionally have console kernel connections always disable comms? We could do this so that if you opened a console, then a notebook associated with the console, the notebook would have comms enabled. |
I think that would be a much more invasive change. This change as-is basically lets the first kernel connection deal with comms, and others have comms disabled. |
It seems that sometimes the defaultKernel wasn’t found in the private list of running kernels…
I've also consolidated some of the testing utilities in this PR. |
CC @blink1073 @ivanov @afshin for review (or anyone else that wants to review this) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
References
Code changes
The comm message protocol currently has implicit assumptions that only
one kernel connection is handling comm messages. This option allows a
kernel connection to opt out of handling comms.
See jupyter/jupyter_client#263
This should fix the error where ipywidgets stops working if you open a notebook and a console to the same kernel, since the console automatically closes all comms since it does not have the comm target registered.
User-facing changes
The first connection to a kernel will handle comm messages, and subsequent connections will ignore comm connections by default. This is better than the status quo, where often subsequent connections would often close comm connections for everyone.
This also prevents many errors in the console from comm messages not being processed in a kernel connection without comm targets. For example, you could see these from the help menu kernel connection, or the console kernel connection.
Backwards-incompatible changes
None. I think all parameters introduced are optional with sensible defaults, and the test suite utility deletions are from a private package.
The
connectToComm
function does throw a new error if the kernel does not handle comms. But its signature hasn't changed in a backwards-incompatible way.