Skip to content

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

Merged
merged 9 commits into from
Apr 26, 2024
Merged

Feat: Add caching support for dependencies #1292

merged 9 commits into from
Apr 26, 2024

Conversation

JoshuaJackson-jobvite
Copy link
Contributor

@JoshuaJackson-jobvite JoshuaJackson-jobvite commented Mar 20, 2024

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?

  • Creates a cache dir
  • Adds in support for creating and restoring caches using action/cache
  • Sets environment variables for digger's execution to pass to the terraform/opentofu setup to leverage the single cache dir for all plugin needs.

@motatoes
Copy link
Contributor

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

@motatoes motatoes closed this Mar 20, 2024
@motatoes motatoes reopened this Mar 20, 2024
@motatoes
Copy link
Contributor

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!

Copy link
Contributor

@motatoes motatoes left a 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

@motatoes
Copy link
Contributor

motatoes commented Apr 22, 2024

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?

JoshuaJackson-jobvite and others added 2 commits April 25, 2024 09:55
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
Copy link
Contributor

@motatoes motatoes left a 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

JoshuaJackson-jobvite and others added 2 commits April 26, 2024 09:26
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
Co-authored-by: Mohamed Habib <moe.habib9@gmail.com>
Copy link
Contributor

@motatoes motatoes left a 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') }}

@motatoes motatoes merged commit 528ad0e into diggerhq:develop Apr 26, 2024
4 checks passed
ben-of-codecraft pushed a commit to ben-of-codecraft/digger that referenced this pull request May 21, 2024
* add caching behaviors as an option to the action
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