feat: Add `cache-key-prefix` option #366

erezrokah · 2023-04-05T13:18:24Z

Description:

Adds a cache key prefix option so it's possible to use different cache keys for different jobs

Related issue:

Fixes #358

Check list:

Mark if documentation changes are required.
Mark if tests were added or updated to cover the changes.

erezrokah · 2023-04-05T13:20:28Z

src/cache-restore.ts

@@ -29,7 +29,8 @@ export const restoreCache = async (
    );
  }

-  const primaryKey = `setup-go-${platform}-go-${versionSpec}-${fileHash}`;
+  const cacheKeyPrefix = core.getInput('cache-key-prefix') || '';


Another option is to use setup-go- as the default

const cacheKeyPrefix = core.getInput('cache-key-prefix') || 'setup-go' const primaryKey = `${cacheKeyPrefix}-${platform}-go-${versionSpec}-${fileHash}`;

erezrokah · 2023-04-06T08:32:58Z

Not sure why the windows tests are failing, so any help will be appreciated

erezrokah · 2023-04-11T11:25:03Z

Working examples in https://github.com/cloudquery/cloudquery/actions/runs/4660341686/jobs/8248290837#step:3:26 and https://github.com/cloudquery/cloudquery/actions/runs/4660341686/jobs/8248291007#step:4:23 via cloudquery/cloudquery#9812

rokerzfirst101 · 2023-04-12T10:47:55Z

Not sure why the windows tests are failing, so any help will be appreciated

I recently forked the repo, made the same changes, and got the same error for the linter workflow. Somehow, rerunning the job with debug logging made it pass.

erezrokah · 2023-05-29T11:48:12Z

Hello lovely maintainers 👋 Anything we can do to move this forward (or close it so we'll know if we need to use a different solution)?

candiduslynx · 2023-05-30T07:17:24Z

Hello lovely maintainers 👋 Anything we can do to move this forward (or close it so we'll if know if we need to use a different solution)?

@dsame @MaksimZhukov @IvanZosimov @marko-zivic-93

marko-zivic-93 · 2023-06-01T06:43:56Z

Hello everyone,

Sorry for the late response, first of all 🙂
We have started working on this feature, but right now we have shifted our focus a bit from it. This should be handled in the upcoming quarter. Thank you for your patience 🙂

dsame · 2023-06-01T06:50:31Z

Hello @erezrokah , can you please review and apply the changes i requested in order we can approve the PR merge?

candiduslynx · 2023-06-01T07:37:22Z

Hello @erezrokah , can you please review and apply the changes i requested in order we can approve the PR merge?

@dsame did you submit the review, though? I can't see any comments ATM

erezrokah · 2023-06-01T08:03:58Z

Hello @erezrokah , can you please review and apply the changes i requested in order we can approve the PR merge?

Thanks for the review @dsame, but I'm not seeing it. Can you confirm you submitted it?

dsame · 2023-06-02T11:08:48Z

@erezrokah please take a look at this one https://github.com/actions/setup-go/pull/366/files/acb2fd804097c1dffc2eb1075b3696da4b4bb27c#r1158511364 ?

candiduslynx · 2023-06-02T11:12:02Z

@erezrokah please take a look at this one https://github.com/actions/setup-go/pull/366/files/acb2fd804097c1dffc2eb1075b3696da4b4bb27c#r1158511364 ?

@dsame
You definitely didn't publish the review (kindly check that you push submit review in the review tab)

erezrokah · 2023-06-02T11:30:38Z

@erezrokah please take a look at this one https://github.com/actions/setup-go/pull/366/files/acb2fd804097c1dffc2eb1075b3696da4b4bb27c#r1158511364 ?

I'm not seeing anything there. Could it be that your review comments are still pending and you need to submit them like @candiduslynx mentioned? See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request#submitting-your-review

dsame · 2023-06-02T11:34:00Z

@candiduslynx @erezrokah thanks for you note, i missed to click "submit" indeed. No the review is published

src/cache-restore.ts

Co-authored-by: Alex Shcherbakov <[email protected]>

erezrokah · 2023-06-02T12:57:49Z

@candiduslynx @erezrokah thanks for you note, i missed to click "submit" indeed. No the review is published

Thanks @dsame, I updated the PR and requested another review

dsame · 2023-06-02T13:51:49Z

@erezrokah can you please run npm run format and commit the format fixes? It is necessary to make these checks to pass: https://github.com/actions/setup-go/actions/runs/5155716632/jobs/9286746499?pr=366

erezrokah · 2023-06-02T14:50:33Z

@erezrokah can you please run npm run format and commit the format fixes? It is necessary to make these checks to pass: https://github.com/actions/setup-go/actions/runs/5155716632/jobs/9286746499?pr=366

Woops, sorry about that. Fixed in ab4b1f7

andig · 2023-06-02T15:42:32Z

Would be great to get a point release once this great PR is in to get cache keys plus tzst compression :) Thank you all for your work!

erezrokah · 2023-06-08T09:16:44Z

Hi 👋 Anything blocking this from getting merged?

andig · 2023-06-15T16:27:21Z

Friendly ping- seems PR is stuck once more?

candiduslynx · 2023-06-15T17:09:51Z

@dsame @MaksimZhukov @IvanZosimov @marko-zivic-93

andig · 2023-06-16T17:59:30Z

PR is done and has three approvals :)

dsame · 2023-06-21T11:11:03Z

@erezrokah @andig while the PR is perfect but there's a hesitation about should it be merged because it repeats functionality of https://github.com/actions/cache action. Please let us a bit more time for discussing.

andig · 2023-06-23T18:58:34Z

@dsame Thank you for taking the time to carefully review!
I feel it does not repeat actions/cache functionality, but rather exposes it. If easier/clearer, you might potentially consider exposing cache-key directly instead of prefix. In any case, I think having to configure an additional key or prefix is much simpler from user perspective than setting up cache with correct folders, even if fully documented.

erezrokah · 2023-07-13T18:07:15Z

I'm happy to close this PR and open a new one to add relevant docs per #358 (comment). Please let me know if you're open to such a contribution

dmitry-shibanov · 2023-07-13T23:13:48Z

Hello @erezrokah. Sorry for the late response. You can prepare a note to the documentation.

erezrokah · 2023-07-14T06:22:42Z

Closing this PR. I'll open a new one for the docs

andig · 2023-07-14T08:07:32Z

I'm really sad this decision had to be made:

In any case, I think having to configure an additional key or prefix is much simpler from user perspective than setting up cache with correct folders, even if fully documented.

We're trading a 2-line zero risk diff against setup complexity for every single user with more than the basic use case. That is imho a bad, high-cost trade-off.

erezrokah requested a review from a team as a code owner April 5, 2023 13:18

erezrokah force-pushed the feat/add_cache_prefix branch from 339ae12 to ecd541e Compare April 5, 2023 13:19

feat: Add cache-key-prefix option

089c1d9

erezrokah force-pushed the feat/add_cache_prefix branch from ecd541e to 089c1d9 Compare April 5, 2023 13:19

erezrokah commented Apr 5, 2023

View reviewed changes

This was referenced Apr 5, 2023

feat: add cache prefix #358

Open

fix(workflows): Disable Go cache for non-test jobs in AWS workflow cloudquery/cloudquery#9656

Closed

Update README.md

acb2fd8

mohammed90 mentioned this pull request Apr 7, 2023

Bump actions/setup-go from 3 to 4 caddyserver/xcaddy#138

Merged

erezrokah mentioned this pull request Apr 9, 2023

bug(aws,gcp,k8s,azure): Test caching in CI does not work cloudquery/cloudquery#9388

Closed

1 task

kolyshkin mentioned this pull request Apr 10, 2023

cache key do not include linux distro name and version #368

Closed

5 tasks

mohammed90 mentioned this pull request Apr 15, 2023

chore: Adjustments to CI caching caddyserver/caddy#5495

Merged

candiduslynx reviewed Jun 2, 2023

View reviewed changes

src/cache-restore.ts Outdated Show resolved Hide resolved

erezrokah and others added 3 commits June 2, 2023 15:46

Update src/cache-restore.ts

847b9da

Co-authored-by: Alex Shcherbakov <[email protected]>

fix: Update dist

cb7e1d2

docs: Update docs and action.yml

3c24beb

erezrokah requested a review from dsame June 2, 2023 12:57

erezrokah requested a review from candiduslynx June 2, 2023 12:57

candiduslynx approved these changes Jun 2, 2023

View reviewed changes

style: Format

ab4b1f7

dsame approved these changes Jun 2, 2023

View reviewed changes

candiduslynx approved these changes Jun 2, 2023

View reviewed changes

marko-zivic-93 approved these changes Jun 7, 2023

View reviewed changes

erezrokah closed this Jul 14, 2023

erezrokah deleted the feat/add_cache_prefix branch July 14, 2023 06:22

erezrokah restored the feat/add_cache_prefix branch July 14, 2023 06:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add `cache-key-prefix` option #366

feat: Add `cache-key-prefix` option #366

erezrokah commented Apr 5, 2023 •

edited

Loading

erezrokah Apr 5, 2023

dsame Jun 1, 2023

erezrokah commented Apr 6, 2023

erezrokah commented Apr 11, 2023

rokerzfirst101 commented Apr 12, 2023

erezrokah commented May 29, 2023 •

edited

Loading

candiduslynx commented May 30, 2023

marko-zivic-93 commented Jun 1, 2023

dsame commented Jun 1, 2023

candiduslynx commented Jun 1, 2023

erezrokah commented Jun 1, 2023

dsame commented Jun 2, 2023

candiduslynx commented Jun 2, 2023 •

edited

Loading

erezrokah commented Jun 2, 2023

dsame commented Jun 2, 2023

erezrokah commented Jun 2, 2023

dsame commented Jun 2, 2023

erezrokah commented Jun 2, 2023

andig commented Jun 2, 2023

erezrokah commented Jun 8, 2023

andig commented Jun 15, 2023

candiduslynx commented Jun 15, 2023

andig commented Jun 16, 2023

dsame commented Jun 21, 2023 •

edited

Loading

andig commented Jun 23, 2023

erezrokah commented Jul 13, 2023

dmitry-shibanov commented Jul 13, 2023

erezrokah commented Jul 14, 2023

andig commented Jul 14, 2023

feat: Add cache-key-prefix option #366

feat: Add cache-key-prefix option #366

Conversation

erezrokah commented Apr 5, 2023 • edited Loading

erezrokah Apr 5, 2023

Choose a reason for hiding this comment

dsame Jun 1, 2023

Choose a reason for hiding this comment

erezrokah commented Apr 6, 2023

erezrokah commented Apr 11, 2023

rokerzfirst101 commented Apr 12, 2023

erezrokah commented May 29, 2023 • edited Loading

candiduslynx commented May 30, 2023

marko-zivic-93 commented Jun 1, 2023

dsame commented Jun 1, 2023

candiduslynx commented Jun 1, 2023

erezrokah commented Jun 1, 2023

dsame commented Jun 2, 2023

candiduslynx commented Jun 2, 2023 • edited Loading

erezrokah commented Jun 2, 2023

dsame commented Jun 2, 2023

erezrokah commented Jun 2, 2023

dsame commented Jun 2, 2023

erezrokah commented Jun 2, 2023

andig commented Jun 2, 2023

erezrokah commented Jun 8, 2023

andig commented Jun 15, 2023

candiduslynx commented Jun 15, 2023

andig commented Jun 16, 2023

dsame commented Jun 21, 2023 • edited Loading

andig commented Jun 23, 2023

erezrokah commented Jul 13, 2023

dmitry-shibanov commented Jul 13, 2023

erezrokah commented Jul 14, 2023

andig commented Jul 14, 2023

feat: Add `cache-key-prefix` option #366

feat: Add `cache-key-prefix` option #366

erezrokah commented Apr 5, 2023 •

edited

Loading

erezrokah commented May 29, 2023 •

edited

Loading

candiduslynx commented Jun 2, 2023 •

edited

Loading

dsame commented Jun 21, 2023 •

edited

Loading