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

Default caching breaks for libraries #199

Closed
ryanhiebert opened this issue Dec 20, 2024 · 7 comments · Fixed by #200
Closed

Default caching breaks for libraries #199

ryanhiebert opened this issue Dec 20, 2024 · 7 comments · Fixed by #200

Comments

@ryanhiebert
Copy link

I don't check in a requirements.txt or a uv.lock because uv is an implementation detail of how I run and test my library.

I think I could work around this by setting the cache key to look at pyproject.toml, but I'm wondering if maybe this is a use-case that you want to support by default?

ryanhiebert/django-cmd#14

@eifinger
Copy link
Collaborator

Thank you for reporting this!
We did not include pyproject.toml because this file can change for several reasons other than changing dependencies.

But it shouldn't break the build. I will tune it down to a warning.

@ryanhiebert
Copy link
Author

Yeah, and it won't change even when there are new version dependencies, so in this case I guess it would be better to skip the caching.

Thanks for being so responsive!

@eifinger
Copy link
Collaborator

Instead of throwing an error it will log a warning.
For the caching behavior this will lead to a cache that never gets invalidated. This might be okay and a speed increase or might not be okay. Users will have to find this out for their workflow and define the cache-dependency-glob accordingly or disable caching.

When I look at your workflow I think it will make a difference of +-1s if at all

@ryanhiebert
Copy link
Author

I'm not too worried about any timing differences, I'm more worried about caching being too aggressive and installing old dependencies, but I'm not sure if that's a concern with uv, especially under tox-uv.

@ryanhiebert
Copy link
Author

I'd rather use the caching as long as uv will still check the versions. It helps out the hosting provider, I think that's a great reason to have a local cache by default. If it still checked the versions from the server while using the cache, then I don't even think there's be anything to warn about.

@eifinger
Copy link
Collaborator

The warning informs users that the cache never gets invalidated and might grow beyond limits the users feels comfortable with. As the cache only contains built wheels this won't be a problem in most cases and the cache won't grow beyond a few MB

@ryanhiebert
Copy link
Author

Oh, that's good. I'm happy with that behavior since I can delete the cache whenever I need to. I'll need to figure out what the default pattern is and just make it explicit, it sounds like. Not too bad, wonder if it could be easier. Or maybe I can bust the cache every month or something, that'd be pretty cool. Thanks for making this change so that I think about the impact my caching will have on the CDN bandwidth that I'm getting for free.

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 a pull request may close this issue.

2 participants