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

ci: cache conda envs betweeen successive builds in a day #3525

Closed
wants to merge 2 commits into from

Conversation

MridulS
Copy link
Contributor

@MridulS MridulS commented May 16, 2022

Quicker turn around time for github actions. The cache is invalidated every day once (date is used in the key for the cache). It will also rerun if there is any change to setup.py.

There is a similar setup for hvplot.

@codecov
Copy link

codecov bot commented May 16, 2022

Codecov Report

Merging #3525 (980ac2c) into master (48281ee) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3525      +/-   ##
==========================================
+ Coverage   82.76%   82.82%   +0.06%     
==========================================
  Files         199      199              
  Lines       27339    27360      +21     
==========================================
+ Hits        22627    22661      +34     
+ Misses       4712     4699      -13     
Impacted Files Coverage Δ
panel/widgets/tables.py 88.77% <0.00%> (+0.01%) ⬆️
panel/widgets/slider.py 84.35% <0.00%> (+0.24%) ⬆️
panel/io/__init__.py 84.21% <0.00%> (+10.52%) ⬆️
panel/io/django.py 26.53% <0.00%> (+26.53%) ⬆️

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 48281ee...980ac2c. Read the comment docs.

Copy link
Member

@maximlt maximlt left a comment

Choose a reason for hiding this comment

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

Thanks for that! 🚀 Just a few suggestions to have caching closer to what has been set on hvplot.

Comment on lines +41 to +47
include:
- os: ubuntu-latest
path: /usr/share/miniconda3/envs/
- os: macos-latest
path: /Users/runner/miniconda3/envs/
- os: windows-latest
path: C:\Miniconda3\envs\
Copy link
Member

Choose a reason for hiding this comment

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

I've seen that these absolute paths have been removed from the documentation of the setup-miniconda action. Maybe they're no longer required?

uses: actions/cache@v2
with:
path: ${{ matrix.path }}
key: ${{ runner.os }}-conda-${{ matrix.python-version }}-${{ hashFiles('setup.py') }}-${{ env.TODAY }}-${{ env.CACHE_VERSION }}
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to have the installed group extras added in the cache key, as done in hvplot:

https://github.com/holoviz/hvplot/blob/04f6dae12c449987abf69601be47c1e292e1ab3a/.github/workflows/test.yaml#L61

We may use caching for the docs workflow for instance (we sometimes trigger many docs build in a day, that could help), and need to make sure the tests key not the same as the docs key. Maybe instead of adding the extras group it'd make more sense to add a hash of the test.yaml file itself?

Comment on lines +85 to +86
restore-keys: |
${{ runner.os }}-conda-${{ matrix.python-version }}-${{ env.TODAY }}-${{ env.CACHE_VERSION }}
Copy link
Member

Choose a reason for hiding this comment

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

In hvplot we had removed that, because of the different workflow types we may run in a single day.

@@ -99,6 +121,7 @@ jobs:
run: |
eval "$(conda shell.bash hook)"
conda activate test-environment
python -m pip install --no-deps --no-build-isolation -e .
Copy link
Member

Choose a reason for hiding this comment

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

Could you move this to its own step before the doit env_capture step? I like to have the doit env_capture step super simple, that's a least one step I know is not causing any issue when I debug the CI 🙃

@philippjfr
Copy link
Member

Thanks for this, it provided a good template. We've now implemented caching as part of a composite action: https://github.com/pyviz-dev/holoviz_tasks/tree/main/install

@philippjfr philippjfr closed this Aug 19, 2022
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.

3 participants