Skip to content

lab default only if available, change filepath definition #1383

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

Merged
merged 3 commits into from
Sep 24, 2021

Conversation

minrk
Copy link
Member

@minrk minrk commented Sep 21, 2021

Changes ?filepath definition from "open with current UI" to "open with classic notebook" to match apparent user expectations.

two pieces of #1368 :

  • opening a file with jupyterlab uses ?labpath instead of ?filepath. filepath is permanently frozen to mean /tree/... with the classic UI
  • selecting 'open a file' in the form behavior is unchanged, in that it produces a ?labpath url, not a filepath url
  • move default url choice down one level into singleuser.cmd so that we can set lab as the default url if jupyterlab 3 is available

If no filepath is specified, and lab 3 is available, lab is still the default UI, leaving it the only link with changed behavior.

@betatim proposed reserving lab-by-default to /v3/ to make the UI change fully explicit and opt-in. I think that if we go all the way to defining a v3 url, we should also change the server launch command to jupyter lab instead of jupyter notebook, because we know that change will be significantly more disruptive, and is also going to happen eventually. It's also a lot simpler to say "v3 starts jupyterlab" than the UI is changed, but the server is not.

We specifically chose this 'only switch default ui' to avoid breaking existing links, so if we are making new links, I think we should update the server command to the current standard, too, rather than having to do this all over again in a few months.

I think we can also communicate a lot better (and take some steps) to address the fact that UI customizations (extensions and such) are very often broken by repo2docker updates, because repo2docker sets the UI versions, not the user. The user may pin down UI, but this can break things, too. There is no good answer for now for stable links with extensions that don't need upkeep by the user.

fully overrideable with singleuser.cmd, singleuser.defaultUrl
- old filepath urls still open classic UI
- form produces new `?labpath=` arg when a file path is chosen

lab is still 'default', but choice is frozen at link-creation time instead of at visit-time
@minrk
Copy link
Member Author

minrk commented Sep 21, 2021

looks like pre-commit ci is running, despite no config (#1381)

@consideRatio
Copy link
Member

I've now disabled pre-commit.ci again in https://github.com/organizations/jupyterhub/settings/installations/17782968/update!

Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

I support the suggested intentions, and the code changes looks good to me. I've raised a question and made a code suggestion about a comment but otherwise I consider this ready for merge.

hackfin added a commit to hackfin/cyrite.howto that referenced this pull request Sep 23, 2021
Co-authored-by: Erik Sundell <[email protected]>
Copy link
Member

@consideRatio consideRatio left a comment

Choose a reason for hiding this comment

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

LGTM! Not sure about the test failure though.

@minrk
Copy link
Member Author

minrk commented Sep 24, 2021

test failure was a DNS failure to resolve discourse.jupyter.org, the kind of once-in-a-while CI networking issue that just happens sometimes. I don't think it's related. Relaunched anyway, though.

@consideRatio
Copy link
Member

I'd like to merge this directly if tests succeed. I believe it is a very relevant fix and we see above that it has been referenced as something awaited as a fix.

@minrk
Copy link
Member Author

minrk commented Sep 24, 2021

all green

@consideRatio consideRatio merged commit c08d5db into jupyterhub:master Sep 24, 2021
@consideRatio
Copy link
Member

Thank you so much for your work on this @minrk!!! ❤️ 🎉

consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request Sep 24, 2021
jupyterhub/binderhub#1383 Merge pull request #1383 from minrk/detect-lab-default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants