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

Enable cache on the pipeline #1729

Closed
wants to merge 1 commit into from
Closed

Enable cache on the pipeline #1729

wants to merge 1 commit into from

Conversation

Kludex
Copy link
Member

@Kludex Kludex commented Oct 26, 2022

There was a previous discussion about this: encode/httpx#1680

One of the goals is to reduce the time on the pipeline, the other is to reduce the amount of networking resources. 🙏

There were no arguments against this, only a note (my own) about the fact that we always need to install the latest dependencies from the setup.py (now pyproject.toml).

The thing is that the cache entry is removed (if not used) after 7 days, see here. Also, once a month we have updates via Dependabot on the requirements.txt, which triggers a new cache entry. Which means once a month we'll have the latest uvicorn dependencies - I think it's reasonable.

@Kludex Kludex requested a review from tomchristie October 26, 2022 19:51
@Kludex Kludex mentioned this pull request Oct 27, 2022
13 tasks
@tomchristie
Copy link
Member

Hrm.

Problem with dependancy caching is that it tends to make it more awkward to track down dependancy breakages when they do occur. Let's consider humans time as the priority here...

Do the sometimes faster test results benefit us more than the "was this with or without the cache? we'd like to test with a flushed cache. etc."

My inclination would be that it's probably not an overall benefit to us as a team, from that perspective.

What do we think?

@Kludex
Copy link
Member Author

Kludex commented Oct 31, 2022

I'm perfectly fine with that reasoning. 🙏

More than anything, I wanted to have a closure on this matter.

Thanks @tomchristie 👍

@Kludex Kludex closed this Oct 31, 2022
@Kludex Kludex deleted the pipeline/enable-cache branch October 31, 2022 10:59
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.

2 participants