-
Notifications
You must be signed in to change notification settings - Fork 562
Feat: Add caching support for dependencies #1292
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
Conversation
We have an issue with our other PR event listerns I need to fix, going to close and re-open the PR to trigger rest of tests |
we can ignore the failing cli e2e test it is happening due to a missing secret in external contribution, I need to figure out a better way to write this test. All in all it looks good , I'm going to perform a test from the branch later and then merge it if all good 👍 thanks for your contribution! |
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.
left you a comment regarding mismatched quote to update
So I fixed the two syntax errors above and did my initial basic test in this repo however it was not successful unfortunately. The first job managed to save the cache successfully. However the subsequent job failed to save the cache with already exists error: https://github.com/motatoes/test-digger-caching/actions/runs/8789860900/job/24120563671 I wonder if we are missing a flag to override the cache? |
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
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.
Starting to follow along now, left you a last set of comments and if I understand it correctly can ship it
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
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.
just one last thing to change "digger-${{ hashFiles('/cache') }}" -> digger--cache-${{ hashFiles('/cache') }}
* add caching behaviors as an option to the action
Why do we need this change?
When running across multiple projects with github actions, the current behaviors are downloading the plugins each time for each project. This pr adds in a behavior to add a caching directory for the terraform and opentofu related behaviors. This should result in a improvement where multiple projects have the same dependency. Between run's as well we'll restore a cache and should prevent the needs to download again across multiple behaviors.
There's a few catches in this initial mvp pr. There's no cleanup of old version of plugins from the cache so it can grow. Multiple executions could fail to write to the cache if they result in the same hash, but as potential issues come up we can address them separately
This work is based on request in #811
What effects does this change have?