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

allow handlers to work without an authorizer in the Tornado settings #717

Merged
merged 1 commit into from Mar 14, 2022

Conversation

Zsailer
Copy link
Member

@Zsailer Zsailer commented Mar 10, 2022

Fixes #713.

We worked on this issue during contributing hour.

Allows the handlers in jupyter_server to be used without its tornado web app by setting the authorizer to None when it's not found in the tornado settings. Adding this to maintain backwards compatibility for now.

It also raises a warning to inform libraries like Voila that they should include an authorizer in the settings in future releases of Jupyter Server.

@Zsailer
Copy link
Member Author

Zsailer commented Mar 11, 2022

This PR raises the warning every time an authorizer isn't found. Ideally, this would only happen the first time it gets called, but we couldn't get this working within the contributing hour time block. I'll try to carve out time to finish this later today, unless anyone wants to pick up the task.

We will likely need this merged by Monday to unblock #716 (see #696 (comment))

@blink1073
Copy link
Collaborator

Kicking CI

@blink1073 blink1073 closed this Mar 14, 2022
@blink1073 blink1073 reopened this Mar 14, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2022

Codecov Report

Merging #717 (1a51c0d) into main (426a4c6) will decrease coverage by 0.04%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #717      +/-   ##
==========================================
- Coverage   70.84%   70.79%   -0.05%     
==========================================
  Files          62       62              
  Lines        7490     7496       +6     
  Branches     1187     1188       +1     
==========================================
+ Hits         5306     5307       +1     
- Misses       1829     1833       +4     
- Partials      355      356       +1     
Impacted Files Coverage Δ
jupyter_server/auth/decorator.py 78.94% <37.50%> (-11.38%) ⬇️
jupyter_server/base/handlers.py 65.72% <100.00%> (ø)
jupyter_server/serverapp.py 65.38% <0.00%> (-0.13%) ⬇️
jupyter_server/extension/application.py 74.66% <0.00%> (+0.34%) ⬆️

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 426a4c6...1a51c0d. Read the comment docs.

Copy link
Collaborator

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

Thanks!

@blink1073 blink1073 closed this Mar 14, 2022
@Zsailer
Copy link
Member Author

Zsailer commented Mar 14, 2022

@blink1073 did you mean to close this without merging? I think we should merge and release this as soon as possible to handle #713.

@Zsailer Zsailer reopened this Mar 14, 2022
@blink1073 blink1073 merged commit deb3d90 into jupyter-server:main Mar 14, 2022
@Zsailer Zsailer deleted the handle-no-authorizer branch January 16, 2024 21:48
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.

Master branch: KeyError on self.settings["authorizer"]
3 participants