Skip to content

use env config path for system config path when no system config path #430

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 4 commits into from
May 27, 2025

Conversation

minrk
Copy link
Member

@minrk minrk commented May 26, 2025

SYSTEM_CONFIG_PATH hasn't technically been guaranteed to be non-empty, but jupyter-server has assumed it is: jupyter-server/jupyter_server#1527 and in practice, it ~always has been until 5.8 because the empty case was weird and rare.

We already use sys.prefix for SYSTEM_JUPYTER_PATH in a similar situation, so it's probably simplest to do the same in config path.

cc @krassowski

I can do a 5.8.1 with this.

@minrk minrk added the bug label May 26, 2025
@krassowski
Copy link
Member

I'm not sure if this will not have any unintended side-effects, but it sounds like it's a better situation than having jupyter server extension not work at all.

We already use sys.prefix for SYSTEM_JUPYTER_PATH in a similar situation, so it's probably simplest to do the same in config path.

I could not find that maybe I am just too tired), can you leave a reference for posterity?

@minrk
Copy link
Member Author

minrk commented May 27, 2025

Sure, the same condition is here for Jupyter path:

SYSTEM_JUPYTER_PATH = [str(Path(sys.prefix, "share", "jupyter"))]

@minrk
Copy link
Member Author

minrk commented May 27, 2025

I looked around, and jupyter-server/jupyter_server#1527 is the only reference to SYSTEM_CONFIG_PATH I can find that would be sensitive to this. All readers of this path rely on jupyter_config_path which handles this and avoids duplicate entries (with a test verifying it), so I think this should be safe.

@minrk minrk merged commit 0d225fd into jupyter:main May 27, 2025
30 checks passed
@minrk minrk deleted the default-system-path branch May 27, 2025 07:23
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 this pull request may close these issues.

2 participants