Skip to content

Speed up GitHub workflows#486

Merged
dixonjoel merged 17 commits intomainfrom
users/bkeryan/poetry-caching
Oct 30, 2023
Merged

Speed up GitHub workflows#486
dixonjoel merged 17 commits intomainfrom
users/bkeryan/poetry-caching

Conversation

@bkeryan
Copy link
Copy Markdown
Collaborator

@bkeryan bkeryan commented Oct 28, 2023

What does this Pull Request accomplish?

  • Caching improvements
    • Cache in-project virtualenvs. All of our caching code was still using ~/.cache/pypoetry/virtualenvs.
    • I originally tried using actions/setup-python's built-in caching, but I ran into two problems:
      • It requires installing Poetry before Python (e.g. with pipx), and when I did this, tests used the wrong Python version. (pipx is slightly faster than Gr1N/setup-poetry, though.)
      • Its cache key setup is limited to simple use cases and can't handle optional extras or Poetry dependency groups.
    • Use restore/save for workflows that run poetry install multiple times with different extras and/or dependency groups.
    • Cache example poetry.lock files. These lock files are not checked into Git, but generating them takes over 5 minutes, so they are worth caching.
    • For system tests, cache the .tox directory as well. Caching on the self-hosted runner has significant overhead, but it seems to be an overall win compared to installing from PyPI for each Python version.
    • Remove caching from Publish_NIMS.yml. We don't run this workflow very often and it's better to avoid a possible source of failure.
  • Disable unit tests on macOS
    • Running tests on macOS often takes longer than on Linux and Windows
    • Caching virtualenvs on macOS was returning errors about invalid/corrupted virtualenvs. Maybe this was because I was skipping poetry install on cache hits at the time, but I don't want to spend any more time on it.

Why should this Pull Request be merged?

Speed up builds (when there is a cache hit):

  • Check NIMG: 1 min -> 30 sec
  • Check NIMS: 2 min 30 sec -> 1 min 15 sec
  • Check examples: 12-15 min -> 2 min 30 sec
  • Run unit tests (Linux): 3 min -> 2 min
  • Run unit tests (Windows): 4-6 min -> 3-4 min
  • Run system tests: 21-23 min -> 15-16 min (not counting time to scale up new runner instance)
  • Total run time: 2 hr -> 1 hr
  • Total duration: 40 min -> 22 min

What testing has been done?

Ran PR workflow.

Comment thread .github/workflows/run_system_tests.yml
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Oct 28, 2023

Test Results

       30 files   -      10         30 suites   - 10   38m 14s ⏱️ - 9m 33s
     479 tests ±       0       479 ✔️ ±       0         0 💤 ±    0  0 ±0 
12 290 runs   - 3 820  11 230 ✔️  - 3 280  1 060 💤  - 540  0 ±0 

Results for commit af161a1. ± Comparison against base commit e7b2b6d.

♻️ This comment has been updated with latest results.

@bkeryan bkeryan force-pushed the users/bkeryan/poetry-caching branch from d67fd6f to 4cab108 Compare October 28, 2023 18:12
`poetry install` is fast when the dependencies are already installed.
Also, if the venv was relocated to a different path, it may fix the venv.
@bkeryan bkeryan marked this pull request as ready for review October 28, 2023 19:06
@bkeryan bkeryan requested a review from mshafer-NI October 28, 2023 19:26
@dixonjoel dixonjoel merged commit cc3191e into main Oct 30, 2023
@dixonjoel dixonjoel deleted the users/bkeryan/poetry-caching branch October 30, 2023 14:31
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