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

Remove RedirectHandler #6997

Merged
merged 1 commit into from
Aug 4, 2023
Merged

Remove RedirectHandler #6997

merged 1 commit into from
Aug 4, 2023

Conversation

jtpio
Copy link
Member

@jtpio jtpio commented Aug 2, 2023

Fixes #6954

Investigate #6954.

Jupyter Server already provides a redirect handler which looks at the default_url here:

https://github.com/jupyter-server/jupyter_server/blob/c5dc0f696f376e1db5a9a0cbcebb40a0bf98875c/jupyter_server/serverapp.py#L440-L451

Which means the one in notebook can likely be removed.

Testing with:

npm i -g configurable-http-proxy
jupyterhub --config jupyterhub_config.py

jupyterhub_config.py:

c = get_config()  # noqa

from jupyterhub.auth import DummyAuthenticator

c.JupyterHub.authenticator_class = DummyAuthenticator

from jupyterhub.spawner import SimpleLocalProcessSpawner

c.JupyterHub.spawner_class = SimpleLocalProcessSpawner
c.JupyterHub.bind_url = 'http://0.0.0.0:8000'
c.Spawner.default_url = '/tree'

@jtpio jtpio added this to the 7.0.x milestone Aug 2, 2023
@jtpio jtpio added the bug label Aug 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

Binder 👈 Launch a Binder on branch jtpio/notebook/default-url

@jtpio
Copy link
Member Author

jtpio commented Aug 2, 2023

The default_url seems to be respected properly when testing with a local jupyterhub:

jupyterhub-default-url.mp4

@jtpio
Copy link
Member Author

jtpio commented Aug 2, 2023

cc @shaneknapp since you opened #6954.

In case you would be able to test this change and see if it fixes #6954.

@shaneknapp
Copy link

cc @shaneknapp since you opened #6954.

In case you would be able to test this change and see if it fixes #6954.

testing it now!

@shaneknapp
Copy link

woohoo! it works!

image

@jtpio
Copy link
Member Author

jtpio commented Aug 3, 2023

Nice thanks @shaneknapp for testing!

@afshin I see you requested a review. Would you like to have a look as well? Otherwise this could likely go in and I'll make a patch release right after.

@jtpio
Copy link
Member Author

jtpio commented Aug 4, 2023

Getting this one in and following-up with a release.

@jtpio jtpio merged commit 24c7624 into jupyter:main Aug 4, 2023
22 checks passed
@jtpio jtpio deleted the default-url branch August 4, 2023 06:28
@shaneknapp
Copy link

thanks SO MUCH for fixing this and rolling out a quick release!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

z2h/notebook 7.0.0rc2 users redirected to tree even though defaultUrl is set to lab
2 participants