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
Fix external-oauth example jupyterhub_config.py #4550
Conversation
- Roles need to be explicitly granted, otherwise you get a 403. This example predates roles. - Explicitly set bind_url - without this, JupyterHub itself doesn't seem to bind anywhere, and so you just get a 404 when you visit whatever port configurable-http-proxy lands on. This is probably a separate bug to be investigated, but in the meantime copying this from testing/jupyterhub_config.py makes this example actually work - Set DummyAuthenticator as the default, so users can get started with this example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 after moving the test boilerplate to the bottom
without this, JupyterHub itself doesn't seem to bind anywhere
That would definitely be a separate bug, but I'm not sure there is one here. This config is setting the bind URL for the proxy itself. The Hub process or what is proxied has not been affected. The default is http://:8000
, which binds to all available interfaces. My guess is that perhaps you had something running on that port already on 127.0.0.1, and binding to ''
weirdly succeeds if there are any other services bound to more specific interfaces, and only gets requests not matched by another, more specific listening socket.
So you can do:
listen('127.0.0.1', port)
listen('1.2.3.4', port)
listen('', port)
and all will succeed, but the ''
will not get any requests unless you make requests to an ip not already bound by another one.
@@ -1,5 +1,16 @@ | |||
import os | |||
|
|||
# Allow anyone to authenticate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In some other examples that have this, I've put it at the bottom to separate it as clearly the "this isn't relevant to the topic of the example, but is needed to make the example runnable". I think that makes it easier to "stop reading here..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@minrk I've moved it to the bottom!
9f7fc24
to
3865df7
Compare
for more information, see https://pre-commit.ci
403. This example predates roles.