Pass session_id during Websocket connect #1440
Conversation
| @@ -47,6 +47,8 @@ async def connect(self): | |||
| url_escape(self.kernel_id), | |||
| "channels", | |||
| ) | |||
| if self.session_id: | |||
| ws_url += f"?session_id={self.session_id}" | |||
There was a problem hiding this comment.
Should session_id be escaped with escape_url? Also a test case would be nice :)
There was a problem hiding this comment.
@krassowski thanks for the feedback, used url_escape and added a test
There was a problem hiding this comment.
Added a test, but looks like Windows tests are failing for unrelated reason?
There was a problem hiding this comment.
The windows tests were fixed in #1441
There was a problem hiding this comment.
Thanks @blink1073 I synced to main and Windows tests work now, but now I get some coverage error. https://github.com/jupyter-server/jupyter_server/actions/runs/9821413970/job/27117179741?pr=1440 any pointers?
Error: Failed to ListArtifacts: Unable to make request: ECONNRESET
If you are using self-hosted runners, please make sure your runner has access to all GitHub endpoints: https://docs.github.com/en/actions/hosting-your-own-runners/managing-self-hosted-runners/about-self-hosted-runners#communication-between-self-hosted-runners-and-github
Thanks
for more information, see https://pre-commit.ci
|
I kicked the coverage test, it looks like it was a temporary connectivity issue. |
|
@krassowski would you mind taking another look? Thanks! |
There was a problem hiding this comment.
Looks good to me. It might be worth bringing it up on the server and kernels meeting tomorrow to check for other ideas.
|
@krassowski is it ok to merge? Thanks! |
|
It looks like @Zsailer commented during the meeting, maybe he can approve and merge (I do not have merge rights here). |
There was a problem hiding this comment.
LGTM. Thanks @gogasca!
Pinging @lresende here too, since he's most familiar with the gateways. Luciano, do you know if e.g. enterprise_gateway tracks individual kernel (ZMQ connection) sessions in any way?
Note, this is different than "sessions" from the server's sessions API, which maps documents to kernels.
The sessions we are dealing with in this PR are tracking ZMQ client sessions. This flows a unique session_id from the browser-->server-->kernel's session client (i.e. at the ZQM interface). I think (?) that JupyterLab uses this type of session_id to distinguish messages coming from the same kernel to different documents, but I'm not totally sure.
We use the word "session" in too many places 😅
|
@Zsailer thanks! do you know when is the next |
Fixes: #1439
Pass HTTP query param session_id during Websocket channel connection.
Example:
vs