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

Accept and manage cookies when requesting gateways #969

Merged
merged 5 commits into from
Sep 13, 2022

Conversation

wjsi
Copy link
Contributor

@wjsi wjsi commented Sep 10, 2022

This PR adds an accept_cookies argument to GatewayClient class. Once enabled, GatewayClient accepts and manages cookie expiration when receiving cookies generated by load balancers serving notebook gateways. These cookies will be sent back in request headers for load balancers to decide which backend node to use.

Purposes and discussions in #967.

@welcome
Copy link

welcome bot commented Sep 10, 2022

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2022

Codecov Report

Merging #969 (0e460ba) into main (4a34e6a) will increase coverage by 0.19%.
The diff coverage is 89.79%.

❗ Current head 0e460ba differs from pull request most recent head e86d938. Consider uploading reports for the commit e86d938 to get more accurate results

@@            Coverage Diff             @@
##             main     #969      +/-   ##
==========================================
+ Coverage   72.30%   72.49%   +0.19%     
==========================================
  Files          64       64              
  Lines        8136     8185      +49     
  Branches     1358     1371      +13     
==========================================
+ Hits         5883     5934      +51     
+ Misses       1843     1839       -4     
- Partials      410      412       +2     
Impacted Files Coverage Δ
jupyter_server/gateway/gateway_client.py 76.84% <89.79%> (+5.65%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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.

Hi @wjsi - this looks good - I just had the one suggestion.

If you could also update with main, that would be great. Thank you.

jupyter_server/gateway/gateway_client.py Outdated Show resolved Hide resolved
@wjsi wjsi force-pushed the enh/stickiness_cookie branch from 0e460ba to e86d938 Compare September 13, 2022 00:19
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.

LGTM - thank you @wjsi!

I suspect the test failure is unrelated, but holding off on the merge until we can confirm tomorrow.

@blink1073 blink1073 merged commit 9a2708e into jupyter-server:main Sep 13, 2022
@welcome
Copy link

welcome bot commented Sep 13, 2022

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@blink1073
Copy link
Contributor

I suspect the test failure is unrelated

Yep, thanks @wjsi and @kevin-bates!

@wjsi wjsi deleted the enh/stickiness_cookie branch September 14, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants