-
Notifications
You must be signed in to change notification settings - Fork 310
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 default_url redirect with default 'main' handler #72
Fix default_url redirect with default 'main' handler #72
Conversation
CC @Zsailer |
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.
Thanks, @gjenkins8. This is a reasonable fix to me.
The MainHandler
really is just a dummy handler for headless servers to avoid redirect errors that are caused by hitting a URI with no handler. Server extensions that expose a frontend should replace the default_url
and redirect the server to hit their own handlers. You're right that this bug was preventing that redirect from happening.
Merging.
Thanks! @Zsailer |
I'm a little confused by what is meant with a "server extension" here. I'm used to it meaning something installed with |
@vidartf It means an extension of jupyter_server, which should be a Python class derived from here: |
@rolweber Thanks for the link. That seems to be an installer class? As far as I understand, there are these components:
If you have multiple server extension, should they all have their own |
@vidartf we've add a new (4th) compontent for creating extensions in #48 , This was specifically built for frontend applications like JupyterLab, notebook, nteract_on_jupyter, and voila. From the perspective of jupyter_server, these applications become "server extensions", i.e. the server exists separate from these applications and they append extra handlers to the server. Those handlers happen to include a frontend application (see more info in #48). JupyterLab, notebook, etc. should no longer subclass ServerApp, but instead, subclass the |
Yes, each extension should define their own Here are two cases that happen now:
The current handler for the |
Thanks for helping me understand the nuances in the terminology!
So, these
Am I getting it right? If so:
Or am I still confused? |
@vidartf everything you said is exactly right. 😃 Sorry for the total lack of documentation. I'm working on this... |
The expected behavior of
ServerApp.default_url
is to redirect to (base_url +)default_url
when the base url (/
) is requested.But I think #54 broke this by adding a
/
handler. Which seems to prevent the redirect handler from working.To fix, I propose conditionally setting the root url handler based on whether
default_url
is specified in config.To reproduce:
jupyter server --ServerApp.default_url='/please_redirect_here' --ServerApp.token='' --ServerApp.open_browser=False
, and navigate tolocalhost:8888
.Note: the
--ServerApp.open_browser=False
option is required, asopen_browser
emulates the redirect logic here. I'll propose removing this logic in a separate PR.