Skip to content
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

Cache: Increasing client validation to 10GB #934

Merged

Conversation

aparna-ravindra
Copy link
Contributor

@aparna-ravindra aparna-ravindra commented Nov 12, 2021

The toolkit contains a check on the client side to ensure that the cache artifact being uploaded is atmost 5GB in size. This PR aims at increasing the size limit to 10GB instead.
The ADR for this change can be found here.

@bishal-pdMSFT
Copy link
Contributor

Please update the README as well:

Note that GitHub will remove any cache entries that have not been accessed in over 7 days. There is no limit on the number of caches you can store, but the total size of all caches in a repository is limited to 5 GB. If you exceed this limit, GitHub will save your cache but will begin evicting caches until the total size is less than 5 GB.

@bishal-pdMSFT bishal-pdMSFT requested a review from dhadka November 15, 2021 10:29
@bishal-pdMSFT
Copy link
Contributor

LGTM to me, but holding back on the sign off to make sure that README is updated.

@aparna-ravindra aparna-ravindra marked this pull request as ready for review November 15, 2021 15:53
@aparna-ravindra aparna-ravindra requested a review from a team November 15, 2021 15:53
Copy link
Member

@dhadka dhadka left a comment

Choose a reason for hiding this comment

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

LGTM!

@aparna-ravindra
Copy link
Contributor Author

Hey @dhadka , The npm audit check has been failing. Is it okay to tackle it separately and not include it in this PR?

@dhadka
Copy link
Member

dhadka commented Nov 17, 2021

@aparna-ravindra re: "The npm audit check has been failing. Is it okay to tackle it separately and not include it in this PR?", I would check with the actions-service team to see if they plan to do this as that audit is running against the entire toolkit and not just this module. But I do agree it makes sense to tackle it separately.

@aparna-ravindra aparna-ravindra merged commit 45d2019 into main Nov 19, 2021
@aparna-ravindra aparna-ravindra deleted the users/aparna-ravindra/cache-client-check-data-limit branch November 19, 2021 11:04
Chocobo1 added a commit to Chocobo1/setup-ccache-action that referenced this pull request Nov 23, 2021
Now it defaults to ccache default 5G.

This is to provide better cache behavior with upstream change:
actions/toolkit#934
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.

3 participants