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 taxonomies in pr workflow #10952

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

McCio
Copy link
Contributor

@McCio McCio commented Oct 30, 2024

cache taxonomies in pr workflow

@github-actions github-actions bot added 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies 📦 Packaging https://wiki.openfoodfacts.org/Category:Recycling 🧽 Data quality https://wiki.openfoodfacts.org/Quality 🧪 tests GitHub Actions Pull requests that update Github_actions code 📍 Origins Origins are used for Eco-Score computation. We want to have structured origins. Tags 🥗 Ingredients labels Oct 30, 2024
@McCio McCio changed the title ci: cache taxonomies ci: cache taxonomies in pr workflow Oct 30, 2024
@McCio McCio force-pushed the ci/taxonomies-cache branch 5 times, most recently from 08c9635 to 0827b10 Compare October 30, 2024 15:29
@github-actions github-actions bot added the 💥 Merge Conflicts 💥 Merge Conflicts label Oct 30, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 49.04%. Comparing base (dc04d18) to head (3ecf37e).
Report is 730 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10952      +/-   ##
==========================================
- Coverage   49.54%   49.04%   -0.51%     
==========================================
  Files          67       77      +10     
  Lines       20650    22237    +1587     
  Branches     4980     5315     +335     
==========================================
+ Hits        10231    10906     +675     
- Misses       9131     9998     +867     
- Partials     1288     1333      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@McCio McCio marked this pull request as ready for review October 30, 2024 19:08
@McCio McCio requested a review from a team as a code owner October 30, 2024 19:08
@McCio
Copy link
Contributor Author

McCio commented Oct 30, 2024

@alexgarel @stephanegigandet as I noted on slack some time ago, currently the github cache is not used during builds - with this PR I'm using the standard cache from github actions (with a bit of a hack copy/pasting from/to the build_cache volume).
As you can see in the timings of the builds, it's significantly faster. Also for new PRs, it will be using main's cache as start point, so it should make the whole pr builds 30 minutes faster (1h less runtime, from ~1h50m to ~50m) when not rebuilding the taxonomy.

The downpoint is that it isn't usable for local development, as far as I know.
If you prefer to keep the github token approach - feel free to close this PR without merging, I already had fun getting to know some github actions 😃

@github-actions github-actions bot removed the 💥 Merge Conflicts 💥 Merge Conflicts label Oct 31, 2024
name: Prepare build_cache volumes
run: |
proj=po_off
docker volume create \
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to use something like --opt device=./build-cache, i.e. mount the folder directly, to avoid having to copy files to and from the docker volume?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know about that, it seems way better! Will try it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 😄

- if: steps.cache.outputs.cache-hit == 'true'
name: Prepare build_cache volumes
run: |
proj=po_off
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better to move the volume creation into the Makefile so that the existing COMPOSE_PROJECT_NAME variable can be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interesting, I'll look into that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 😄

- name: Download backend image from artifacts
uses: ishworkh/docker-image-artifact-download@v1
uses: ishworkh/container-image-artifact-download@v2.0.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed for this particular change or is it just a general version update?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Kinda: it's just a general version update, as I was originally searching for a ready-to-use action to do the same with docker volume, I noticed that version is deprecated and is using the old github action to upload artifacts.
Updating it I noticed it's a bit faster than the old one, so I left it

@McCio McCio force-pushed the ci/taxonomies-cache branch 2 times, most recently from 558750e to c77fdd7 Compare October 31, 2024 16:05
Copy link

sonarcloud bot commented Oct 31, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧽 Data quality https://wiki.openfoodfacts.org/Quality GitHub Actions Pull requests that update Github_actions code 🥗 Ingredients 📍 Origins Origins are used for Eco-Score computation. We want to have structured origins. 📦 Packaging https://wiki.openfoodfacts.org/Category:Recycling Tags 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies 🧪 tests
Projects
Status: Todo
Status: In progress
Development

Successfully merging this pull request may close these issues.

3 participants