-
Notifications
You must be signed in to change notification settings - Fork 5
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 caching to CI workflow #109
Conversation
3a6245e
to
d5b64f4
Compare
This shaves off 2 minutes (20% of previous CI wall time) for each CI run. |
key: | ||
conda-${{ runner.os }}--${{ runner.arch }}--${{ | ||
hashFiles('ci-environment.yml') }}-${{ | ||
env.DATE }}-${{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want the date to be in the cache key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the date ensure the cache is updated daily to account for any changes in conda package versions. See https://github.com/marketplace/actions/setup-miniconda#caching-environments, which I essentially copied here.
with: | ||
miniforge-variant: Mambaforge | ||
miniforge-version: latest | ||
# Do not specify environment file - see Cache step below |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So at this point, we have an empty environment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the env always be updated in the last step, whether the cache exists or not.
path: ${{ env.CONDA }}/envs | ||
key: | ||
conda-${{ runner.os }}--${{ runner.arch }}--${{ | ||
hashFiles('ci-environment.yml') }}-${{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to throw the conda version in there too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's necessary. The cache is invalidated every 24 hours, so within that time period, if the conda version changes and somehow the resolved set of packages for our ci-environment.yml
changes we can easily invalidate the cache on this page: https://github.com/microsoft/yardl/actions/caches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense.
No description provided.