Skip to content

Caching on GHES - #308

Merged
brcrista merged 7 commits into
actions:mainfrom
tiwarishub:cacheOnGhes
Mar 31, 2022
Merged

Caching on GHES#308
brcrista merged 7 commits into
actions:mainfrom
tiwarishub:cacheOnGhes

Conversation

@tiwarishub

@tiwarishubtiwarishub commented Mar 29, 2022

Copy link
Copy Markdown
Contributor

Description:
This PR adds caching support in setup-java for GHES with version 3.5. It checks the presence of the Actions cache service(which is used for caching dependencies in GHES and dotcom) using the recent @actions/cache toolkit package function i.e isFeatureAvailable. This same function can be applied to dotcom scenario (github.com) so this function can be safely applied to dotcom scenario as well.
I have tested the below scenarios
on GHES
Current behaviour
image

With new changes. Without AC on GHES
image

With AC on GHES
save
image
restore
image

without choosing the cache option on GHES with AC
image

On dotcom

save
image

cache hit
image

without cache option
image

Related issue:
#307

Check list:

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

@tiwarishubtiwarishub changed the title Cache on ghesCaching on GHESMar 30, 2022
@tiwarishub
tiwarishub marked this pull request as ready for review March 30, 2022 06:26
@tiwarishub
tiwarishub requested review from a team and bishal-pdMSFTMarch 30, 2022 06:27
Comment threadpackage.json Outdated
{
"name": "setup-java",
"version": "2.0.0",
"version": "2.0.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to update the version according to the next release version.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we are planning to release after this PR merge

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please change version 2.0.1 to the planing one ?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmitry-shibanov , please share planned version

Comment threadsrc/util.ts Outdated
);
} else {
core.warning(
'An internal error has occurred in cache backend. Please check https://www.githubstatus.com/ for any ongoing issue in actions.'

@brcristabrcristaMar 30, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this message accurate? It looks like cache.isFeatureAvailable() just looks for whether an environment variable is set.

https://github.com/actions/toolkit/blob/b4639928698a6bfe1c4bdae4b2bfdad1cb75016d/packages/cache/src/cache.ts#L53

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my understanding is correct, I would say something like

Suggested change
'An internal error has occurred in cache backend. Please check https://www.githubstatus.com/ for any ongoing issue in actions.'
'This runner is not configured to use the cache service. Caching will be skipped.'

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment threadsrc/util.ts Outdated
);
} else {
core.warning(
'This runner is not configured to use the cache service. Caching will be skipped'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update to the same thing from actions/setup-python#363?

Suggested change
'This runner is not configured to use the cache service. Caching will be skipped'
'The runner was not able to contact the cache service. Caching will be skipped'

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated with correct message

@brcrista

Copy link
Copy Markdown
Contributor

@tiwarishub

tiwarishub commented Mar 31, 2022

Copy link
Copy Markdown
ContributorAuthor

@brcrista
brcrista merged commit dc1a9f2 into actions:mainMar 31, 2022
tdfacer pushed a commit to ifit/setup-java that referenced this pull request Oct 7, 2025
* initial changes
* review comments
* updated with correct message
* linting
* update version
* updated version
Sign up for freeto 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.

5 participants

@tiwarishub@brcrista@dmitry-shibanov@marko-zivic-93@aparna-ravindra