tahoe-lafs/tahoe-lafs
tahoe-lafs
/
tahoe-lafs
2
0
Fork 0
Code Issues Pull Requests 23 Packages Projects Releases 1 Wiki Activity

3591 Use pip cache in GitHub Actions #967

Merged
sajith merged 12 commits from 3591.use-pip-cache-in-github-actions into master 2021-01-20 14:57:38 +00:00
Conversation 2 Commits 12 Files Changed 2 +53 -9
sajith commented 2021-01-18 16:03:40 +00:00 (Migrated from github.com)
Copy Link

Ticket is https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3591.

Some notes:

  • We're using actions/cache here. Python-specific caching action requires pip 20.1+ because it implements pip cache dir command to get pip's cache directory in an OS-independent manner. There's no perceivable overall speedup in CI completion time because of caching (possibly because we're in a noisy shared environment), but we'd be hammering pypi much less often than before, which is nice.

  • Since we have steps containing pip install --upgrade codecov tox setuptools, we'd be still downloading some packages from PyPI. We probably can do a pip install codecov tox setuptools instead, thus using cached versions of those packages, but I guess tracking their latest versions could be useful, because any breakage with them would surface early on.

  • Piggy-backing on this PR is the removal of git fetch --prune --unshallow step, and addition of fetch-depth: 0 parameter to actions/checkout. Step with the former usually takes 10-20s, so avoiding that is nice.

What's the point of doing some (allegedly) pointless caching without some (almost pointless) numbers? Well, we do have some numbers!

There's some meaningful difference in "installing Python packages" step, where we do a pip install --upgrade codecov tox setuptools for coverage tests, and pip install --upgrade tox for integration and packaging tests. Comparing the tip of this PR branch (where caching enabled) versus base of this branch (where caching action is essentially a no-op):

installing packages without caching with caching
coverage (macos) 1m 24s 12s
coverage (windows) 21s 19s
integration (macos) 11s 6s
integration (windows) 21s 9s
packaging (macos) 45s 6s
packaging (windows) 20s 9s
packaging (ubuntu) 9s 6s

And here are the overall CI completion times, which are kind of all over the place:

without caching with caching
coverage (macos) 18m 57s 13m 55s
coverage (windows) 14m 3s 16m 43s
integration (macos) 4m 33s 3m 23s
integration (windows) 6m 34s 5m 26s
packaging (macos) 3m 56s 5m 20s
packaging (windows) 6m 9s 5m 59s
packaging (ubuntu) 1m 40s 1m 28s

Looks like the time-consuming step is the actual running of tests.

Ticket is https://tahoe-lafs.org/trac/tahoe-lafs/ticket/3591. Some notes: * We're using [actions/cache](https://github.com/actions/cache/blob/main/examples.md#python---pip) here. Python-specific caching action requires pip 20.1+ because it implements `pip cache dir` command to get pip's cache directory in an OS-independent manner. There's no perceivable overall speedup in CI completion time because of caching (possibly because we're in a noisy shared environment), but we'd be hammering pypi much less often than before, which is nice. * Since we have steps containing `pip install --upgrade codecov tox setuptools`, we'd be still downloading some packages from PyPI. We probably can do a `pip install codecov tox setuptools` instead, thus using cached versions of those packages, but I guess tracking their latest versions could be useful, because any breakage with them would surface early on. * Piggy-backing on this PR is the removal of `git fetch --prune --unshallow` step, and addition of `fetch-depth: 0` parameter to `actions/checkout`. Step with the former usually takes 10-20s, so avoiding that is nice. What's the point of doing some (allegedly) pointless caching without some (almost pointless) numbers? Well, we do have some numbers! There's some meaningful difference in "installing Python packages" step, where we do a `pip install --upgrade codecov tox setuptools` for coverage tests, and `pip install --upgrade tox` for integration and packaging tests. Comparing the tip of this PR branch (where caching enabled) versus base of this branch (where caching action is essentially a no-op): | installing packages | without caching | with caching | :-----------------------|-----------------|--------------| | coverage (macos) | 1m 24s | 12s | | coverage (windows) | 21s | 19s | | integration (macos) | 11s | 6s | | integration (windows) | 21s | 9s | | packaging (macos) | 45s | 6s | | packaging (windows) | 20s | 9s | | packaging (ubuntu) | 9s | 6s | And here are the overall CI completion times, which are kind of all over the place: | | without caching | with caching | |-----------------------|-----------------|--------------| | coverage (macos) | 18m 57s | 13m 55s | | coverage (windows) | 14m 3s | 16m 43s | | integration (macos) | 4m 33s | 3m 23s | | integration (windows) | 6m 34s | 5m 26s | | packaging (macos) | 3m 56s | 5m 20s | | packaging (windows) | 6m 9s | 5m 59s | | packaging (ubuntu) | 1m 40s | 1m 28s | Looks like the time-consuming step is the actual running of tests.
codecov[bot] commented 2021-01-18 16:56:51 +00:00 (Migrated from github.com)
Copy Link

Codecov Report

Merging #967 (ed92202) into master (cd06f1c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #967   +/-   ##
======================================
  Coverage      92%     92%           
======================================
  Files         151     151           
  Lines       27000   27000           
  Branches     4042    4042           
======================================
  Hits        24931   24931           
  Misses       1427    1427           
  Partials      642     642

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd06f1c...ed92202. Read the comment docs.

# [Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967?src=pr&el=h1) Report > Merging [#967](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967?src=pr&el=desc) (ed92202) into [master](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/commit/cd06f1c902a237c3cf30fcbca2a0555d0ae86557?el=desc) (cd06f1c) will **not change** coverage. > The diff coverage is `n/a`. [![Impacted file tree graph](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967/graphs/tree.svg?width=650&height=150&src=pr&token=Ztmu9sr4vi)](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967?src=pr&el=tree) ```diff @@ Coverage Diff @@ ## master #967 +/- ## ====================================== Coverage 92% 92% ====================================== Files 151 151 Lines 27000 27000 Branches 4042 4042 ====================================== Hits 24931 24931 Misses 1427 1427 Partials 642 642 ``` ------ [Continue to review full report at Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967?src=pr&el=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967?src=pr&el=footer). Last update [cd06f1c...ed92202](https://codecov.io/gh/tahoe-lafs/tahoe-lafs/pull/967?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
jaraco (Migrated from github.com) approved these changes 2021-01-20 02:41:49 +00:00
jaraco (Migrated from github.com) left a comment
Copy Link

LGTM.

Regarding the fetch-depth 0, I suspect that in the future some applications may require some depth (such as to resolve recent tags), but until that's needed, this approach seems suitable.

LGTM. Regarding the fetch-depth 0, I suspect that in the future some applications may require some depth (such as to resolve recent tags), but until that's needed, this approach seems suitable.
sajith commented 2021-01-20 14:55:17 +00:00 (Migrated from github.com)
Copy Link

LGTM.

Regarding the fetch-depth 0, I suspect that in the future some applications may require some depth (such as to resolve recent tags), but until that's needed, this approach seems suitable.

Thank you @jaraco!

Per actions/checkout documentation, a fetch-depth of 0 will fetch history for all tags and all branches. Tahoe-LAFS needs more git metadata than what's checked out by the default fetch-depth of 1 to be able to populate src/allmydata/_version.py, without which allmydata.test.test_client.Basic.test_versions will break.

> LGTM. > > Regarding the fetch-depth 0, I suspect that in the future some applications may require some depth (such as to resolve recent tags), but until that's needed, this approach seems suitable. Thank you @jaraco! Per [actions/checkout](https://github.com/actions/checkout) documentation, a `fetch-depth` of 0 will fetch history for all tags and all branches. Tahoe-LAFS needs more git metadata than what's checked out by the default `fetch-depth` of 1 to be able to populate `src/allmydata/_version.py`, without which `allmydata.test.test_client.Basic.test_versions` will break.
Sign in to join this conversation.
Reviewers
No reviewers
jaraco
Labels
Clear labels   
Benchmarking and Performance

   
HTTP Storage Protocol

   
Nevow Removal

   
Python 3 Porting

   
not-for-merge
No Label
Benchmarking and Performance
HTTP Storage Protocol
Nevow Removal
Python 3 Porting
not-for-merge
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: tahoe-lafs/tahoe-lafs#967
No description provided.
Delete Branch "3591.use-pip-cache-in-github-actions"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?