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 user_data_dir rather than user_runtime_dir for view notifications #761

Merged
merged 2 commits into from
Oct 25, 2024

Conversation

jjallaire-aisi
Copy link
Collaborator

Resolves #665

It seems as if unlike other directories, user_runtime_dir does not have a fallback if the user don't have the requisite parent directories with permissions. This causes failure modes on devcontainers and other minimally configured systems.

The resolution is to use the user_data_dir instead, which has a ~/.local/share fallback on Linux. VS Code extension is also updated to look for view notification files in both locations to support older and newer versions.

Copy link

@OlyDSIT OlyDSIT left a comment

Choose a reason for hiding this comment

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

Looks good, thanks JJ!

@jjallaire-aisi jjallaire-aisi merged commit a428520 into main Oct 25, 2024
10 checks passed
@jjallaire-aisi jjallaire-aisi deleted the feature/user-data-dir branch October 25, 2024 15:58
@art-dsit
Copy link
Contributor

Resolves #665

It seems as if unlike other directories, user_runtime_dir does not have a fallback if the user don't have the requisite parent directories with permissions. This causes failure modes on devcontainers and other minimally configured systems.

The resolution is to use the user_data_dir instead, which has a ~/.local/share fallback on Linux. VS Code extension is also updated to look for view notification files in both locations to support older and newer versions.

I assume you mean user_data_path, per the code in this PR

There is unfortunate inconsistency in naming between XDG and platformdirs.

https://platformdirs.readthedocs.io/en/latest/api.html

has

user_data_dir - data directory tied to the user
user_data_path - data path tied to the user

XDG has

$XDG_DATA_HOME defines the base directory relative to which user-specific data files should be stored. If $XDG_DATA_HOME is either not set or empty, a default equal to $HOME/.local/share should be used.

$XDG_DATA_DIRS defines the preference-ordered set of base directories to search for data files in addition to the $XDG_DATA_HOME base directory. The directories in $XDG_DATA_DIRS should be separated with a colon ':'.

If $XDG_DATA_DIRS is either not set or empty, a value equal to /usr/local/share/:/usr/share/ should be used.

Assuming XDG_DATA_DIRS = user_data_dir, we wouldn't want to use that, since it looks like it would be read-only

If XDG_DATA_HOME = user_data_dir, then we are good to use user_data_dir.

If the upstream bug is fixed, will we revert this to simplify things?

@jjallaire-aisi
Copy link
Collaborator Author

The _dir vs. _path distinction is just one returns a string and one returns a Path

XDG_DATA_DIRS is a fallback list of other root directories to search within (in addition to .local).

I don't think I'll revert if the upstream bug if fixed b/c it will make us depend on a super recent version of platformdirs (which could possibly cause problems for other dependency resolution). runtime_dir is a super obscure variation (again, MacOS and Windows don't even have it) so I think a poor choice in the first place.

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.

[bug] Inspect fails to run in the vs code python 3 devcontainer
5 participants