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

Make the kernel_websocket_protocol flag reusable. #1264

Merged
merged 2 commits into from May 10, 2023

Conversation

ojarjur
Copy link
Contributor

@ojarjur ojarjur commented Apr 25, 2023

This change moves the kernel_websocket_protocol flag out from the sub-class ZMQChannelsWebsocketConnection and into its base class BaseKernelWebsocketConnection so that it is automatically available to any other sub classes of BaseKernelWebsocketConnection.

This change moves the `kernel_websocket_protocol` flag out from
the sub-class `ZMQChannelsWebsocketConnection` and into its base
class `BaseKernelWebsocketConnection` so that it is automatically
available to any other sub classes of `BaseKernelWebsocketConnection`.
Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this change - thank you Omar. (Sorry the delayed response!)

@blink1073 blink1073 merged commit ecd2822 into jupyter-server:main May 10, 2023
34 of 35 checks passed
ojarjur added a commit to ojarjur/jupyter_server that referenced this pull request May 10, 2023
…socketConnection

This change removes a temporary work-around to the required `kernel_ws_protocol`
field not being defined in the `BaseKernelWebsocketConnection` base class.

Previously, that field was only defined in the ZMQChannelsWebsocketConnection class,
so we had to reproduce it in the `GatewayWebsocketConnection` class in order to
get that class to work correctly. However, we only provided a minimal default value
of `None` rather than properly supporting the user setting their preferred websocket
protocol.

This workaround was made unnecessary by jupyter-server#1264 which moved the `kernel_ws_protocol`
field from the ZMQChannelsWebsocketConnection class to the base class.

Accordingly, this commit removes that temporary work-around that is no longer needed.
We needed to provide a value for that,
@ojarjur ojarjur deleted the websocket-protocol-flag branch May 10, 2023 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants