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

Refactor combined cache. #16039

Closed
wants to merge 1 commit into from
Closed

Conversation

coeuvre
Copy link
Member

@coeuvre coeuvre commented Aug 3, 2022

Instead of guessing when to use remote/disk part, combined cache now uses read/write cache policy provided by the context. The call sites can change the policy based on the requirements.

Fixes #15934. But unfortunately, I am not able to add an integration test for it because our own remote worker doesn't support the asset API.

@coeuvre coeuvre requested a review from a team as a code owner August 3, 2022 12:50
@coeuvre coeuvre requested a review from tjgq August 3, 2022 12:52
@sgowroji sgowroji added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Aug 3, 2022
Instead of guessing when to use remote/disk part, combined cache now uses read/write cache policy provided by the context. The call sites can change the policy based on the requirements.

Fixes bazelbuild#15934. But unfortunately, I am not able to add an integration test for it because our own remote worker doesn't support the asset API.
@sgowroji sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Aug 4, 2022
@copybara-service copybara-service bot closed this in d55c331 Aug 5, 2022
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 5, 2022
@sgowroji
Copy link
Member

sgowroji commented Aug 7, 2022

@bazel-io fork 5.3.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 7, 2022
ShreeM01 added a commit that referenced this pull request Aug 8, 2022
Instead of guessing when to use remote/disk part, combined cache now uses read/write cache policy provided by the context. The call sites can change the policy based on the requirements.

Fixes #15934. But unfortunately, I am not able to add an integration test for it because our own remote worker doesn't support the asset API.

Closes #16039.

PiperOrigin-RevId: 465577383
Change-Id: I99effab1cdcba0890671ea64c4660ea31b059ce7

Co-authored-by: Chi Wang <[email protected]>
@ShreeM01
Copy link
Contributor

Hi @coeuvre, we are trying to [cherry-pick] (#16073) this change to the release 5.3.0. But pre-submits checks are failing due to missing dependencies. So could you please help us in cherry-picking this change with appropriate commits? Thanks!

coeuvre added a commit to coeuvre/bazel that referenced this pull request Aug 16, 2022
Instead of guessing when to use remote/disk part, combined cache now uses read/write cache policy provided by the context. The call sites can change the policy based on the requirements.

Fixes bazelbuild#15934. But unfortunately, I am not able to add an integration test for it because our own remote worker doesn't support the asset API.

Closes bazelbuild#16039.

PiperOrigin-RevId: 465577383
Change-Id: I99effab1cdcba0890671ea64c4660ea31b059ce7
@coeuvre
Copy link
Member Author

coeuvre commented Aug 16, 2022

cherry pick is ready #16110.

ShreeM01 pushed a commit that referenced this pull request Aug 16, 2022
Instead of guessing when to use remote/disk part, combined cache now uses read/write cache policy provided by the context. The call sites can change the policy based on the requirements.

Fixes #15934. But unfortunately, I am not able to add an integration test for it because our own remote worker doesn't support the asset API.

Closes #16039.

PiperOrigin-RevId: 465577383
Change-Id: I99effab1cdcba0890671ea64c4660ea31b059ce7
@ShreeM01 ShreeM01 removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--noremote_accept_cached cannot be used with remote downloader as of Bazel 5.0
5 participants