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

Set CONDA_STORE_DIR to platformdirs.user_data_path #786

Merged
merged 13 commits into from
Apr 5, 2024

Conversation

nkaretnikov
Copy link
Contributor

@nkaretnikov nkaretnikov commented Mar 17, 2024

Fixes #648.

Description

This pull request:

  • sets CONDA_STORE_DIR to platformdirs.user_data_path
  • updates docs
  • adds a test.

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Additional information

How to test

Copy link

netlify bot commented Mar 17, 2024

Deploy Preview for conda-store canceled.

Name Link
🔨 Latest commit 03059d7
🔍 Latest deploy log https://app.netlify.com/sites/conda-store/deploys/661036d97725510008c1a09f

@nkaretnikov nkaretnikov changed the title Set CONDA_STORE_DIR to platformdirs.user_data_dir Set CONDA_STORE_DIR to platformdirs.user_data_path Mar 17, 2024
@nkaretnikov nkaretnikov marked this pull request as ready for review March 18, 2024 04:29
@nkaretnikov nkaretnikov requested a review from jaimergp March 18, 2024 04:30
@nkaretnikov
Copy link
Contributor Author

@jaimergp This is ready and tested, PTAL. CI failures are unrelated.

@jaimergp
Copy link
Member

This looks good. Let's discuss the pinning item briefly and then we can make a decision.

@nkaretnikov
Copy link
Contributor Author

@jaimergp Updated, PTAL.

The only concern I have is that the library version bounds might not be strict enough. The authors introduced a breaking change between 2.5.4 and 2.6.0. But this can also be updated later if we run into problems.

@nkaretnikov nkaretnikov requested a review from jaimergp March 22, 2024 12:37
@jaimergp
Copy link
Member

The only concern I have is that the library version bounds might not be strict enough. The authors introduced a breaking change between 2.5.4 and 2.6.0. But this can also be updated later if we run into problems.

Yea, I saw that, but hopefully they have learnt from that oversight. Also this kind of comment makes me believe they are being more careful now and not too keen on breaking anything else.

Copy link
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

Just one more thing.

Also, since this adds a new dependency, what's the process for letting the release manager know there are new deps to be added to the conda recipe? Will they look at a specific file? Maybe pyproject.toml also needs adjusting?

@nkaretnikov
Copy link
Contributor Author

@jaimergp I've added platformdirs to pyproject.toml. I've never released conda-store, let's ask Tania.

@trallard How do new dependencies affect the release process? Anything to do besides updating pyproject.toml and yaml files? I've read release.md -- just making sure.

@trallard
Copy link
Collaborator

Only the pyprojecr.toml needs updating

@nkaretnikov nkaretnikov requested a review from jaimergp March 23, 2024 17:48
@nkaretnikov nkaretnikov merged commit 3b8686d into conda-incubator:main Apr 5, 2024
25 checks passed
@nkaretnikov nkaretnikov deleted the platformdirs-648 branch April 5, 2024 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

[ENH] - Consider using OS-specific standards for conda-store data
3 participants