feat: Add cache-key-prefix option

[erezrokah]

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]

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

[dsame]

const cacheKeyPrefix = core.getInput('cache-key-prefix') || 'setup-go'

const primaryKey = `${cacheKeyPrefix}-${platform}-go-${versionSpec}-${fileHash}`;

[erezrokah]

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

[erezrokah]

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]

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]

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]

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]

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]

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

[candiduslynx]

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]

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]

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

[candiduslynx]

@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]

@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]

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

[erezrokah]

@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]

@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]

@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]

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]

Hi 👋 Anything blocking this from getting merged?

[andig]

Friendly ping- seems PR is stuck once more?

[candiduslynx]

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

[andig]

PR is done and has three approvals :)

[dsame]

@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]

@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]

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]

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

[erezrokah]

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

[andig]

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.