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

Await _connect and inline read_messages callback to _connect #350

Merged
merged 1 commit into from
Nov 21, 2020

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