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

preferred_dir bug #806

Closed
mlucool opened this issue Apr 26, 2022 · 17 comments · Fixed by #826
Closed

preferred_dir bug #806

mlucool opened this issue Apr 26, 2022 · 17 comments · Fixed by #826
Labels

Comments

@mlucool
Copy link

mlucool commented Apr 26, 2022

Description

I'm not sure if this repo is the root cause of this bug, but setting --ServerApp.preferred_dir on the CLI overrides root_dir as well.

Example:

Reproduce

Example (assume cwd /path/to/cwd)

python3 -m venv myvenv --system-site-package
source myvenv/bin/activate
pip install jupyterlab
pip install git+https://github.com/jupyter-server/jupyter_server.git@1.x # clearer error messages

jupyter-lab --ServerApp.preferred_dir=/path/to/preferred

This errors (correctly) because /path/to/preferred is not a subdir of /path/to/cwd.

Now edit a config, such as ~/.jupyter/jupyter_server_config.py and put in:

c.ServerApp.root_dir = '/'
c.ServerApp.preferred_dir=`/path/to/preferred`

This works:

jupyter-lab

This does not as root-dir is overridden back to /path/to/cwd

jupyter-lab --ServerApp.preferred_dir=/path/to/preferred2

Expected behavior

Setting preferred_dir does not change root-dir.

Context

jupyter-server@1.x
jupyterlab@3.3.4

@mlucool mlucool added the bug label Apr 26, 2022
@echarles
Copy link
Member

While using previously the preferred_dir feature, I have always provided both values and it was working fine.

jupyter-lab --ServerApp.preferred_dir=/my_root/preferred2  --ServerApp.root_dir=/my_root

If I only provide preferred_dir with ...

jupyter-lab --ServerApp.preferred_dir=/Users/.../Desktop/

.... it fails with Bad config encountered during initialization: preferred_dir must be equal or a subdir of root_dir. preferred_dir: ''/Users/.../Desktop'' root_dir: ''/Users/.../current_dir'

So if no root_dir is provided, the current directory (cwd) is considered the root_directory, which is I guess the expected behavior.

If I start with

jupyter-lab --ServerApp.preferred_dir=/Users/.../current_dir/docs

the server start fine and jupyter lab shows the home icon (preferred location) as expected.

Screenshot 2022-04-27 at 10 44 20

@kevin-bates
Copy link
Member

I was just about to say that I'm also unable to reproduce this issue, then saw the backticks in Marc's example for the preferred_dir value:

c.ServerApp.root_dir = '/'
c.ServerApp.preferred_dir=`/path/to/preferred`

so, what the heck, I've heard of more bizarre stuff before, and modified my config to use the backticks. Sure enough, this appears to side-affect the calculation of root_dir as it goes from '/' to my cwd.

(Note that the preferred_dir is appropriate no manner which way things are specified and the CLI option appropriately overrides the config option.)

Assuming this is the common link, this feels more like an issue in traitlets than here, since that's the code interpreting these values.

@mlucool
Copy link
Author

mlucool commented Apr 27, 2022

Heh, the backticks were a mistake on my part, not intentionally part of the bug reoirt. I actually am using c.ServerApp.preferred_dir = os.getcwd(). Happy to move this to traitlets if that is where the error most likely is.

@kevin-bates
Copy link
Member

Hmm. Then I guess I can't reproduce your issue when setting the config file entry to:

c.ServerApp.preferred_dir = os.getcwd()

In that case, the configured root_dir (of '/') is honored, irrespective of how preferred_dir is specified (and assuming the latter is a sub-directory of the former).

I think there should be some other reproductions before transferring elsewhere and, if it only amounts to backticks being the issue, I believe there could be justification for push back since they're really not valid string specifiers.

@kevin-bates
Copy link
Member

kevin-bates commented Apr 27, 2022

Also, I'm wondering if there needs to be some further clarification.

How are you determining that root-dir is getting changed/reverted? Is it based on the directory produced from within a running kernel? Or by virtue of what directories can be navigated via the file browser (left-hand) pane? (I'm using the latter of these.)

I still feel like we're missing some information here if you're able to reproduce this.

@mlucool
Copy link
Author

mlucool commented Apr 27, 2022

I am able to reproduce this on multiple operating systems using the directions above. There is no need to put preferred_dir in jupyter_server_config.py - that was just for proving it works when paired.

% cat ~/.jupyter/jupyter_server_config.py 
c.ServerApp.root_dir = '/Users'

% jupyter-lab --ServerApp.preferred_dir=/Users/myusername
[C 2022-04-27 19:41:26.483 ServerApp] Bad config encountered during initialization: preferred_dir must be equal or a subdir of root_dir. preferred_dir: ''/Users/myusername'' root_dir: ''/Users/myusername/WebstormProjects''

How are you determining that root-dir is getting changed/reverted?

https://github.com/jupyter-server/jupyter_server/pull/805/files adds this in the output (which is why I install from 1.x).

@kevin-bates
Copy link
Member

Thanks for the latest information Marc - that is helpful. Looking back on things, I definitely got confused because this is reproducible and I probably had preferred_dir in my config file during those tests, and when not, may have had my cwd between or above the configured root_dir and the CLI preferred_dir (as noted in the "success" rows). (sigh)

root_dir (config) preferred_dir (cli) cwd relative location of cwd result
/Users/myusername /Users/myusername/foo/bar /Users/myusername/foo between success, root_dir = /Users/myusername
/Users/myusername /Users/myusername/foo/bar /Users/myusername/foo/bar/baz below fail, root_dir = /Users/myusername/foo/bar/baz
/Users/myusername /Users/myusername/foo/bar /Users above success, root_dir = /Users/myusername
/Users/myusername /Users/myusername/foo/bar /tmp external fail, root_dir = /tmp

@mlucool
Copy link
Author

mlucool commented Apr 28, 2022

Sorry my initial example was confusing, I should have started with my second one to keep it simpler. Glad others can reproduce this now - that table is interesting. Do you think it's a jupyter_server bug or a traitlets bug?

@echarles
Copy link
Member

I can not reproduce on my laptop with 4.0.0a24 nor 3.3.4. The following starts as expected.

cat ~/.jupyter/jupyter_server_config.py 
c.ServerApp.root_dir = '/Users/echarles'
jupyter lab --ServerApp.preferred_dir=/Users/echarles/Desktop 

Moving the jupyter_server_config.py in the current folder (where I launch jupyter lab) give the same result (starts correctly).

@echarles
Copy link
Member

My last run with jupyterlab 4.0.0a24 has

jupyter-server               1.17.0
traitlets                    5.1.1

@mlucool
Copy link
Author

mlucool commented Apr 28, 2022

@echarles can you shares your CWD when you run these commands?

@echarles
Copy link
Member

echarles commented Apr 28, 2022

mmh when I run the command from the

  • / : OK
  • /Users: OK
  • /Users/echarles: OK
  • /Users/echarles/Desktop: OK
  • /Users/echarles/foo: KO

So running from anywhere in the defined tree /.../root_dir/preferred_dir works fine

And yes, it breaks if you run from outside of that tree. We have a reproducer.

@echarles
Copy link
Member

I realise this is what @kevin-bates has shown with his table.

Do you think it's a jupyter_server bug or a traitlets bug?

Prolly a jupyter_server bug. I will look into that.

@kevin-bates
Copy link
Member

My IDE is not cooperating but, from what I can determine, this is probably an issue with the order traits are validated. My guess is that the default values are gathered (so root_dir == cwd), then the CLI traits are validated (so preferred_dir is validated against the "default" root_dir), then the config traits are validated (so root_dir is updated). The failure cases prevent the last step from occurring, which is why a cwd within, and inclusive of, the endpoints (root_dir ...preferred_dir) works, and anything else does not.

It might be a tough row to hoe to get changes in traitlets (I would presume to defer validation until after configs are loaded) so it might be better to move the validation for preferred_dir to be a function of the validation for root_dir (and forgo this being done by the @validate("preferred_dir") decorator). This way the appropriate value for root_dir should be in place, irrespective of where preferred_dir is defined (and assuming the order within the config file doesn't matter).

@kevin-bates
Copy link
Member

This turned out to be a bit of a rat's nest for such an edge case but will submit a PR shortly.

@mlucool
Copy link
Author

mlucool commented May 2, 2022

Thanks @kevin-bates!

@echarles
Copy link
Member

echarles commented May 3, 2022

Thx a lot @kevin-bates for the fix and associated tests. I have tested latest main and I confirm the issue is solved.

The backport PR to 1.x branch is available on #828

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants