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

Enable using cache registry for the first build #1767

Merged

Conversation

mtparet
Copy link
Contributor

@mtparet mtparet commented Dec 6, 2024

Registry caching was disabled for the first build although it permits to share cache between images. This will speed lot of first builds.

@mtparet mtparet requested a review from a team as a code owner December 6, 2024 09:45
@mtparet
Copy link
Contributor Author

mtparet commented Feb 7, 2025

Hi @chenbh, any chance to get a review ? Thanks 🙏

@chenbh
Copy link
Contributor

chenbh commented Feb 10, 2025

@mtparet it looks like GitHub Actions didn't pick up your commit for whatever reason, can you try regenerating the commit hash and force pushing?

git commit --amend --no-edit
git push -f

Registry caching was disabled for the first build although it permits
to share cache between images. This will speed lot of first builds.
@mtparet mtparet force-pushed the activate_registry_first_build branch from 0bea8d7 to 97cff71 Compare February 11, 2025 06:43
@mtparet
Copy link
Contributor Author

mtparet commented Feb 11, 2025

@chenbh thanks! Just did it but it seems you need to approve the workflow to run here below :

image

If you are comfortable with running workflows on the pull request branch, return to the Conversation tab, and under "Workflow(s) awaiting approval", click Approve and run.

https://docs.github.com/en/actions/managing-workflow-runs-and-deployments/managing-workflow-runs/approving-workflow-runs-from-public-forks#approving-workflow-runs-on-a-pull-request-from-a-public-fork

@chenbh
Copy link
Contributor

chenbh commented Feb 11, 2025

I swear that button wasn't there last time 😅.

@mtparet One of the tests is failing

Adding a function to select it based on the name and
not the place in the array.
@mtparet
Copy link
Contributor Author

mtparet commented Feb 13, 2025

@chenbh I believe you :D

I fixed the test, the way we select the container based on its position in the initContainers array is not very reliable and we were not testing the right container. I added a function to select the container based on the name, I can open a future PR to convert all existing tests so we ensure we test right containers everywhere.

@mtparet
Copy link
Contributor Author

mtparet commented Feb 13, 2025

(I need an approval on the workflow to trigger the test again)

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 35.31%. Comparing base (49014e2) to head (beaa711).
Report is 95 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1767       +/-   ##
===========================================
- Coverage   67.34%   35.31%   -32.03%     
===========================================
  Files         140      257      +117     
  Lines        8886    19964    +11078     
===========================================
+ Hits         5984     7051     +1067     
- Misses       2393    12311     +9918     
- Partials      509      602       +93     

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

@mtparet
Copy link
Contributor Author

mtparet commented Feb 13, 2025

Tests passed :) Is there anything left I should do to get it merged @chenbh ?

@tomkennedy513 tomkennedy513 merged commit 7de12b1 into buildpacks-community:main Feb 13, 2025
1 check passed
@mtparet
Copy link
Contributor Author

mtparet commented Feb 13, 2025

Thanks @tomkennedy513 !

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.

4 participants