Skip to content

Conversation

@ricklamers
Copy link
Contributor

@ricklamers ricklamers commented Nov 21, 2020

This PR should fix an issue with the gateway handler introduced by the switch to native async function for the _connect function in the gateway websocket handler (which originally used the @gen coroutine version from tornado).

_connect should now be properly awaited through IOLoop's spawn_callback mechanism.

@kevin-bates recommended a PR based on this discussion jupyter-server/enterprise_gateway#903.

@codecov-io
Copy link

codecov-io commented Nov 21, 2020

Codecov Report

Merging #350 (457059a) into master (a0640b3) will increase coverage by 0.31%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #350      +/-   ##
==========================================
+ Coverage   67.07%   67.38%   +0.31%     
==========================================
  Files          55       55              
  Lines        5352     5357       +5     
  Branches      696      695       -1     
==========================================
+ Hits         3590     3610      +20     
+ Misses       1555     1543      -12     
+ Partials      207      204       -3     
Impacted Files Coverage Δ
jupyter_server/gateway/handlers.py 32.67% <28.57%> (+0.22%) ⬆️
jupyter_server/_version.py 100.00% <0.00%> (ø)
jupyter_server/serverapp.py 62.08% <0.00%> (+0.58%) ⬆️
jupyter_server/base/handlers.py 67.64% <0.00%> (+0.75%) ⬆️
jupyter_server/gateway/managers.py 77.25% <0.00%> (+1.22%) ⬆️
jupyter_server/log.py 85.18% <0.00%> (+14.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5997a6...457059a. Read the comment docs.

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.

Looks good Rick - thank you!

I've taken these changes for a spin in my EG configuration and they are essential to supporting EG from jupyter-server!

@Zsailer
Copy link
Member

Zsailer commented Dec 2, 2020

@meeseeksdev backport to 1.0.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyter_server that referenced this pull request Dec 2, 2020
kevin-bates added a commit that referenced this pull request Dec 3, 2020
…on-1.0.x

Backport PR #350 on branch 1.0.x (Await _connect and inline read_messages callback to _connect)
hMED22 pushed a commit to hMED22/jupyter_server that referenced this pull request Jan 23, 2023
…-fix

Await _connect and inline read_messages callback to _connect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants