👷 Add Python venv cache

[Kludex]

This PR aims to add a cache to the Python environment.

I also took the audacious move to indent with an empty line between each action, so let me know if you wish for it to be removed.

Discussion on Gitter

@Kludex:

@tomchristie how would you feel about adding a cache on the requirements installation on the test-suite github workflow? I can open a PR on all encode projects if you think it makes sense.

@tomchristie:

Depends. Best place to start would be probably a pull request against httpx.
The cons are the extra fiddliness etc that's being introduced, the pros are that it ought to make the builds faster
So, the smart approach is to introduce it in a pull request, and then observe what the actual difference in build times looks like.

Reference: https://gitter.im/encode/community?at=60c3235cc705e53c1c86be7e

Docs: https://github.com/actions/cache

[Kludex]

Pipeline failing due to mypy.

[Kludex]

The difference in time with and without cache:

References:

• https://github.com/encode/httpx/actions/runs/936461005
• https://github.com/encode/httpx/actions/runs/936471466

[florimondmanca]

@Kludex This is pretty sweet. The time difference when looking at individual jobs is even better:

Job	master	add-env-cache
Python 3.6	1min45s	39s
Python 3.7	1min40s	35s
Python 3.8	1min49s	29s
Python 3.9	1min43s	30s

That's a 4x improvement. :-) Also a more sober approach that's nicer with PyPI and network usage.

I would be happy to take this in, I'll let @tomchristie see if that's "not too fiddly" enough to be worth using across Encode?

[tomchristie]

Where have you gotten ${{ env.pythonLocation }} from?
(I mean, it looks neat, but I don't understand it)

I don't see anything about it mentioned in the docs, here.

[Kludex]

I took it from this tutorial.

But it's defined on the setup-action above. Here specifically: https://github.com/actions/setup-python/blob/dc73133d4da04e56a135ae2246682783cc7c7cb6/src/find-python.ts#L62

[tomchristie]

Should we make sure that we have more strictly pinned dependancies before pulling this in?

Eg... https://github.com/encode/httpcore/pull/184/files

[Kludex]

I'm always in favor of pinning development dependencies.

Do you want to have the same setup on all encode projects i.e. pinning + dependabot?

[florimondmanca]

@Kludex We experimented with Dependabot for HTTPCore. To be honest I'm not entirely convinced we need it, over a regular "bump dependencies" PR once in a while. On HTTPCore Dependabot bumps deps every month, even only for patch releases; those PR need to be merged one at a time, with rebasing between each… I'm not sure we need it.

I'd say we can start with pinning deps, get this in, then decide if we want Dependabot?

[Kludex]

What if we do what is suggested in this article for HTTPCore: https://www.hrvey.com/blog/combine-dependabot-prs

I'll pin the dependencies here in the meantime.

[tomchristie]

So, I wouldn't include the pinning as part of this.
We ought to consider that in a separate pull request.

[Kludex]

I assumed that your question was a requirement for this 😅 We can do that or just invalidate the cache in n days?

[tomchristie]

I assumed that your question was a requirement for this 😅

Possibly. Not absolutely necessary - even with pinning we won't end up with a fully 100% absolutely guaranteed identical installs between runs, because of various edge cases.

Even if we do want to treat it as a requirement, it'd still make sense to PR it separately, since I think that pinning our deps is definitely something we'd like, whereas this pull request is probably something that we'd like.

(Having spent a long time maintaining projects, I'm almost always on the side of keeping PRs well isolated wherever possible.)

[Kludex]

because of various edge cases.

Is there another case than dependencies of dependencies that you're including here?

[Kludex]

Reverted requirements.

[tomchristie]

Is there another case than dependencies of dependencies that you're including here?

Ha, not really - I think I was a bit tired, and couldn't think about describing "transitive dependancies".

It's possible? there's also some edges around eg. deps varying on diff environments, meaning we can't hard-pin some cases? Dunno.

[Kludex]

@Kludex We experimented with Dependabot for HTTPCore. To be honest I'm not entirely convinced we need it, over a regular "bump dependencies" PR once in a while. On HTTPCore Dependabot bumps deps every month, even only for patch releases; those PR need to be merged one at a time, with rebasing between each… I'm not sure we need it.

I'd say we can start with pinning deps, get this in, then decide if we want Dependabot?

@florimondmanca I've tested this GitHub action on this repository. It has a minimal setup, only for demonstration purposes, and it worked very well (please check the closed PRs to understand the timeline).

@tomchristie what do you think about adding this bot?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.