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

Add dependency caching step to tests in CI #1975

Closed
wants to merge 117 commits into from
Closed

Conversation

tanertopal
Copy link
Member

@tanertopal tanertopal commented Jun 28, 2023

Issue

Description

This PR improves the speed of the CI.

Related issues/PRs

This PR assumes #2103 has already been merged. The CI should be triggered couple of times after that has happened to compare the impact of the caching.

Explanation

The CI cache works as follows:

main branch

Whenever a new commit is made to the main branch the build_python_cache job in .github/workflows/ci-cache.yml will run and cache the following directories and files:

  • **/poetry.lock
  • ~/.cache/pip
  • ~/.cache/pypoetry

The cache will be overwritten each time and can be retrieved by jobs in pull requests by the following key:

${{ runner.os }}-python-cache-${{ matrix.python-version }}-${{ hashFiles('**/pyproject.toml') }}

This basically means the job retrieving the cache has to meet the following requirements.

  1. Same OS
  2. Same Python version
  3. No pyproject.toml file in the repository has changed

The consequence is that updates to any pyproject.toml file should usually be made in isolation to save time. As an additional note: the main branch jobs will never use (read) a cache.

Pull Requests

Pull requests will try to use the cache by doing a lookup using the previously described key and pull the cache is available.


Checklist

  • Implement proposed change
  • Write tests
  • Update documentation
  • Update changelog
  • Make CI checks pass
  • Ping maintainers on Slack (channel #contributions)

@tanertopal tanertopal self-assigned this Jun 28, 2023
@tanertopal tanertopal marked this pull request as ready for review June 28, 2023 14:15
@tanertopal tanertopal requested a review from danieljanes as a code owner June 28, 2023 14:15
@danieljanes
Copy link
Member

@tanertopal is there a way to set a retention period for caching the poetry.lock file? I wouldn't want to rely just on the 10GB limit. Also, we probably don't need to delete all caches, deleting just the lock file should be sufficient.

@tanertopal
Copy link
Member Author

tanertopal commented Jun 29, 2023

@tanertopal is there a way to set a retention period for caching the poetry.lock file? I wouldn't want to rely just on the 10GB limit. Also, we probably don't need to delete all caches; deleting just the lock file should be sufficient.

@danieljanes Unfortunatly not. You either built a cache based on a key and number of file paths to cache. If we want to build multiple caches that's possible, although I think it would become messy fast and will create a lot of duplicate code and make the CI definitions less maintainable.

Deleting and rebuilding the caches immediately once a day should be good. It's predictable and cheap. I can add a job for it or do you see any problem with it?

* main:
  Remove line with typo from script (#2001)
  Fix simulation error caused by Pydantic version in Ray (#2002)
  Update mxnet examples dependency versions (#1982)
  Update torch version in pyproject.toml files (#1993)
  Update tensorflow examples dependency versions (#1984)
  Fix the baseline template docstrings (#1983)
  Fix Baselines CI PyLint error by using Resampling (#1998)
  Make tensorflow optional when using tensorboard (#1879)
  Update dependencies for MXNet example (#1988)
  Fix `starlette` vulnerability (#1992)
  Fix vulnerability detected by Dependabot (#1990)
  Fix dependabot critical errors (#1989)
  Add Ruff UP check (#1946)
  Update torch examples dependency versions (#1981)
  Remove && from the end of lines in test.sh (#1979)
  Baselines docs updates (#1977)
  Fixes GitHub CI issue because of behaviour change (#1976)
…e_ci_speed

* 'improve_ci_speed' of github.com:adap/flower:
* main:
  Add new baselines test github workflow (#2009)
  Add simulation to E2E tests (#2074)
  Add E2E test for Pandas (#2070)
  Add E2E test for scikit-learn (#2073)
  Add format and test scripts (#1987)
  Add missing ruff dependency to baselines (#2075)
  Add E2E test for MXNet (#2069)
  Add E2E test for Jax (#2067)
  Update bare E2E test client (#2068)
  Update PyTorch E2E test (#2072)
  Update Tensorflow E2E test (#2071)
  Fix flake8 error E266 in baseline template (#2065)
  Fix baseline creation on linux-based systems (#2063)
  Updates to Baseline Template Readmes (#2059)
  Refresh FedProx MNIST baseline (#1918)
  Extend test checking tools config (#1986)
  Improved documentation (#2006)
  updated material for 30min FL tutorial (#2005)
  Update codeowners list (#2004)
  Create and delete nodes via Fleet API (#1901)
…ptimize_ci_structure

* 'optimize_ci_structure' of github.com:adap/flower:
  Add dependabot to E2E tests (#2082)
description: "Version of pip to be installed using pip"
default: 23.1.2
default: 22.0.2
Copy link
Member Author

@tanertopal tanertopal Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this because its the GitHub hosted cached version and saves some time.

@tanertopal
Copy link
Member Author

Closing in favor of #3480

@tanertopal tanertopal closed this May 22, 2024
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