[Django] #36794: Cache python dependencies on GitHub Actions

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Type:
| Cleanup/optimization
Status: new | Component: Core
| (Other)
Version: dev | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Good task for new-ish contributor:
Make a small survey of how other major python projects cache their
dependencies in their Github Actions workflows, and implement something
for Django.

Note there are several workflows that would make use of this.

Adam Johnson's blog:
https://adamj.eu/tech/2023/11/02/github-actions-faster-python-virtual-
environments/

This would shave about a minute off the test job.
--
Ticket URL: <https://code.djangoproject.com/ticket/36794>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned

Component: Core (Other) | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Nilesh Pahari):

* owner: (none) => Nilesh Pahari
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:1>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Clifford Gama):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:2>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* cc: Pravin (added)
* has_patch: 0 => 1
* needs_better_patch: 0 => 1

Comment:

Nilesh, there's a contributor who is already submitting a pull request for
this, so just be aware, you may wish to restate your intentions or
collaborate with them. Pravin, please accommodate Nilesh if they would
prefer to keep working. Also, you have several other PRs open, not all of
which follow the contribution guidelines for commit & PR titles. It would
be better to focus on bringing those to completion first.

[https://github.com/django/django/pull/20414 PR] (draft)
--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:3>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Nilesh Pahari):

Replying to [comment:3 Jacob Walls]:

> Nilesh, there's a contributor who is already submitting a pull request
for this, so just be aware, you may wish to restate your intentions or
collaborate with them. Pravin, please accommodate Nilesh if they would
prefer to keep working. Also, you have several other PRs open, not all of
which follow the contribution guidelines for commit & PR titles. It would
be better to focus on bringing those to completion first.
>
> [https://github.com/django/django/pull/20414 PR] (draft)

Hi!
I wasn't able to find any other major project using the pattern mentioned
in the Adam Johnson's blog, so I just replicated the pattern (mentioned in
the blog) on all the workflows in order to cache the whole virtual
environment rather than just the python wheels. I have already made the
changes and will be raising a PR shortly. Also just to confirm, the note
about having several open PRs was directed at me?
--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:4>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

Nope, it was not. Sorry for the confusion!
--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:5>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 1 => 0

Comment:

[https://github.com/django/django/pull/20415 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:6>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:7>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Nilesh Pahari):

Workflows are now passing. I also made them more robust by adding a check
to only create the virtual environment when it doesn’t already exist.
--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:8>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Nilesh Pahari):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:9>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari
Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:10>

[Django]

#36794: Cache python dependencies on GitHub Actions
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Nilesh
Type: | Pahari

Cleanup/optimization | Status: closed

Component: Core (Other) | Version: dev

Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Jacob Walls):

* has_patch: 1 => 0

* needs_better_patch: 1 => 0

* resolution: => wontfix
* status: assigned => closed

Comment:

This turned out to be more subtle than I expected.

We ''were'' using `cache: 'pip'`, so we ''were'' availing ourselves of
cached wheels. As detailed on Adam's blog post, the potential optimization
here is about not moving the wheels into place and not checking for
updated dependencies, which an application with exact pins wouldn't need
to do. We don't pin. I didn't have this internalized when writing the
ticket.

Nilesh did a great job applying my feedback, but after the latest round I
realized we had two different patterns to maintain -- one for PRs, and
another on pushes to main to get "canary" testing of breakages in latest
dependencies -- but on top of that, I realized we'd want to evict the
caches at some point. After a month? Write a new cache-eviction job, or
use the month in the cache key?

I ran this by Natalia and we agreed that the complexity/maintenance
tradeoff was starting to get worse here. So our recommendation is to do
nothing for now.

Thanks Nilesh for the investigation.
--
Ticket URL: <https://code.djangoproject.com/ticket/36794#comment:11>