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

👷 Update setup-python action in tests to use new caching feature #5680

Merged
merged 2 commits into from Nov 25, 2022

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Nov 23, 2022

Updates the "Test" GitHub Actions workflow to use the cache feature on the setup-python action instead of the separate cache action.

This should resolve libffi problems failing builds right now e.g. https://github.com/tiangolo/fastapi/actions/runs/3531449114/jobs/5924630998 and simplify future maintenance.

Unfortunately there is not yet upstream support for caching based on the dependency section of a pyproject.toml and there will be a cache miss if unrelated metadata changes. See actions/setup-python#529.

Since FastAPI does not use a requirements.txt file, a file to generate a hash from must be provided to the action
@github-actions
Copy link
Contributor

📝 Docs preview for commit 30ec2d7 at: https://637e7a79cb2392410df8af2c--fastapi.netlify.app

@tiangolo tiangolo changed the title Update setup-python action in tests to use new caching feature 👷 Update setup-python action in tests to use new caching feature Nov 25, 2022
@tiangolo tiangolo merged commit 22837ee into tiangolo:master Nov 25, 2022
@tiangolo
Copy link
Owner

Ha! I didn't know about this new cache. Thank you! 🙇 🍰

@tiangolo
Copy link
Owner

It seems this only caches the files downloaded by pip, but not the installation in the environment. 😔 actions/setup-python#330

So, the normal cache is still needed. I'll add that back in another PR.

@zanieb
Copy link
Contributor Author

zanieb commented Nov 27, 2022

:( @tiangolo sorry about that! Where I've used it elsewhere, we don't skip the installation step based on a cache hit because we want to ensure we're testing against the latest versions of libraries and the cache is just used to speed installation time.

Seems likely that the corrupted cache issue that prompted this can be resolved by just changing the version suffix you had on the cache key in your original implementation. (edit: for future reference, this was done in #5696)

@tiangolo
Copy link
Owner

Ah! I get it! Makes sense. And no worries! It's working now.

Also the docs for that weren't obvious to me, I thought they meant what I was thinking. 😅

But anyway, it's solved now. 🤓☕

@Avasam
Copy link

Avasam commented Dec 2, 2022

Unfortunately there is not yet upstream support for caching based on the dependency section of a pyproject.toml

Hi! I just saw this issue link and would like to offer you a workaround: After checkout, but before the python action step, you can read your pyproject.toml dependency section (I use the SebRollen/toml-action) and output it to a new file. Then you'll have a file to use as the cache-key.

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.

None yet

5 participants