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

use jupyter_config_dir instead of config_path[0] for workspaces, settings #13589

Merged
merged 1 commit into from Dec 16, 2022

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Dec 14, 2022

avoids these being siloed in virtualenvs and conda envs, which is:

  1. rarely desirable, I suspect (I'm sure some follks want this, but they can opt-in with $JUPYTER_CONFIG_DIR/WORKSPACES/etc.)
  2. far more likely to be not writable than user dirs

References

background: #13580
simpler alternative to #13581

Related: jupyter/jupyter_core#318

c l o s e s #13587
closes #13580

This doesn't add the writability check in #13581, but instead adds the explicit requirement that $JUPYTER_CONFIG_DIR is writable, which I think is easier to manage and communicate than trying to track down env paths and inconsistent default behavior of jupyter-core described in jupyter/jupyter_core#318. The current behavior is that jupyterlab has the requirement that environments are writable, which is not a valid assumption.

Code changes

use jupyter_config_dir() (always a user dir unless overridden explicitly) for workspaces/user-settings storage instead of jupyter_config_path()[0]. These are almost always the same, but if you are in a virtualenv or conda environment (including root), the latter will return a path in the env, which may not be writable. I also suspect deviating workspaces and user-settings persistence by env is not desirable by default (I know I would never want this).

User-facing changes

Default storage for workspaces, user-settings will change when jupyterlab is running in an environment with jupyter-core >= 5, from inside the environment to the standard user Jupyter config location. i.e. ~/.jupyter.

Backwards-incompatible changes

Config storage path moving means that previously stored config will not be loaded. Not sure what level of deprecation/migration is considered appropriate here.


Edited: This does not fix the error seen on the example CI job.

…ings

avoids these being siloed in virtualenvs, which is:

1. rarely desirable, I suspect
2. may not be writable
@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@vidartf
Copy link
Member

vidartf commented Dec 14, 2022

Having the settings be shared across envs is probably what most users want. That said, my concern is what happens if the user has different versions of lab in different envs. Most changes to the settings system (either in core or in 3rd party extensions) care about being backwards compatible when users are upgrading, but not compatible when users are downgrading. So if a user has two envs, one with e.g. JupyterLab 4.0 RC, and one env with lab 3.2.1, then there might be schema errors that pop up (which I think will currently just break things?). I think that before we merge this, we should try to make sure we correctly handle such cases. If that is already the case, and I have just missed it, then I would of course be happy :)

On a more subjective note, I find that having a different default workspace for each env can be pretty practical. Basically I get all the notebooks and files that are relevant in that env open automatically! But that extra benefit will be much smaller when we get a workspaces UI #6944.

@vidartf
Copy link
Member

vidartf commented Dec 14, 2022

PS: I will try to find a good example of a non-downgrade-proof setting change that was made in the past..

@krassowski
Copy link
Member

krassowski commented Dec 14, 2022

Based on the above comments (without testing, so I could be wrong): this might break my experience/expectation of opening independent workspace when I switch between projects (I have separate virtual environment for each project).

This is a severe problem for me because the workspace essentially gets erased (=my temporary memory of which notebooks and files I worked on/planned working on does too). The core of the problem is lack of support for "projects", which we could for example resolve as proposed in #12916.

I am not opposed the proposed change in principle since it fixes a bug, but just wanted to ensure that the ability to auto-restore workspaces between projects gets preserved (whether by limiting the extent of proposed change, or implementing ana alternative mechanism to achieve per-project config, again see #12916)

@minrk
Copy link
Contributor Author

minrk commented Dec 15, 2022

@krassowski I think that's absolutely a reasonable use case, and it should be well supported. I think setting $JUPYTER_CONFIG_DIR as part of env activation should do this correctly.

Note that the behavior after this PR was already the behavior prior to jupyter-core 5, so upgrading jupyter_core is what changed this storage location, not anything in jupyterlab.

I think per-env and per-user storage are both perfectly reasonable approaches to support, and there are always going to be folks who wish their preference were the default. I think the most important thing is that the default is reliable and unsurprising, and the second most important thing is that whichever is not the default is easy to opt-in to.

Examples of surprising behavior to avoid:

  • launching lab in an env loses all my user-settings (current behavior)
  • launching lab in an env might mess something up in settings due to a version change (suggested by @vidartf, more likely with this PR). This is also true of any version change within a single env, though.

For opt-in: with the current behavior, env preference can be selected with JUPYTER_PREFER_ENV_PATH. After this PR, it can be selected with JUPYTER_CONFIG_DIR. Both before and after, it can be more specifically controlled with JUPYTERLAB_WORKSPACES/USER_SETTINGS_DIR env, independently of other configuration, such as server config.

I think in general, it's not quite right to use jupyter_config_path() to select a single directory, as done here, since that should mainly be used to load things from a priority path where all dirs on the path are expected to load in a priority order, even though that priority may change. But jupyter-core doesn't specify this, so it's ambiguous.

@fcollonval
Copy link
Member

Thanks a lot @minrk

As avoid a frustrating bug on the user side, I will merge this one, backport it to 3.5.x and carry the release.

@fcollonval
Copy link
Member

Improving/supporting the use case per env or per project, can be done in #12916

@fcollonval fcollonval merged commit f15709f into jupyterlab:master Dec 16, 2022
@fcollonval
Copy link
Member

@meeseeksdev please backport to 3.6.x

@fcollonval
Copy link
Member

@meeseeksdev please backport to 3.5.x

meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request Dec 16, 2022
meeseeksmachine pushed a commit to meeseeksmachine/jupyterlab that referenced this pull request Dec 16, 2022
fcollonval pushed a commit that referenced this pull request Dec 16, 2022
…for workspaces, settings (#13604)

Co-authored-by: Min RK <benjaminrk@gmail.com>
@minrk minrk deleted the jupyter-config-dir branch December 16, 2022 18:23
fcollonval added a commit that referenced this pull request Dec 19, 2022
… config_path[0] for workspaces, settings) (#13605)

* Backport PR #13589: use jupyter_config_dir instead of config_path[0] for workspaces, settings

* Fix Python test

Co-authored-by: Min RK <benjaminrk@gmail.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2023
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.

Attempts to write workspaces to non-writable workspaces dir in shared env
4 participants