Skip to content

[6.0.0] [remote] Respect whether the server supports action cache updates#16724

Merged
meteorcloudy merged 3 commits intobazelbuild:release-6.0.0from
EngFlow:yannic-cp-128d833fee99f8a43bc4de82cbec752e4ce6fb47
Nov 10, 2022
Merged

[6.0.0] [remote] Respect whether the server supports action cache updates#16724
meteorcloudy merged 3 commits intobazelbuild:release-6.0.0from
EngFlow:yannic-cp-128d833fee99f8a43bc4de82cbec752e4ce6fb47

Conversation

@Yannic
Copy link
Copy Markdown
Contributor

@Yannic Yannic commented Nov 9, 2022

Only a subset of users may be allowed to update the action cache (e.g., only CI but not devs).

Today, there are 2 ways to achive the desired behavior:

  • GetCapabilities returning that all users are allowed to update, and UpdateActionResult returning an error that Bazel prints and ignores, or
  • have the users that are not allowed to update the action cache set --remote_upload_local_results=false.

Why don't we instead respect whether the remote cache supports uploading action results?

Note that this requires support from the remote system to fully work (i.e., it needs to return update_enabled = false for users that don't have permission). Otherwise, Bazel's behavior will be the same as before this change: failed UpdateActionResult do not cause the build to fail. The only change this introduces is that Bazel will no longer error if --remote_upload_local_results=true and GetCapabilities returning update_enabled = false.

Closes #16624.

PiperOrigin-RevId: 486901751
Change-Id: I0991f6891e21711df1e23ae0998a8bc95e2389bc

…ates

Only a subset of users may be allowed to update the action cache (e.g., only CI but not devs).

Today, there are 2 ways to achive the desired behavior:
  - `GetCapabilities` returning that all users are allowed to update, and `UpdateActionResult` returning an error that Bazel prints and ignores, or
  - have the users that are not allowed to update the action cache set `--remote_upload_local_results=false`.

Why don't we instead respect whether the remote cache supports uploading action results?

Note that this requires support from the remote system to fully work (i.e., it needs to return `update_enabled = false` for users that don't have permission). Otherwise, Bazel's behavior will be the same as before this change: failed `UpdateActionResult` do not cause the build to fail. The only change this introduces is that Bazel will no longer error if `--remote_upload_local_results=true` and `GetCapabilities` returning `update_enabled = false`.

Closes bazelbuild#16624.

PiperOrigin-RevId: 486901751
Change-Id: I0991f6891e21711df1e23ae0998a8bc95e2389bc
@Yannic Yannic requested a review from ShreeM01 as a code owner November 9, 2022 19:32
@Yannic
Copy link
Copy Markdown
Contributor Author

Yannic commented Nov 9, 2022

@meteorcloudy @kshyanashree PTAL

@ShreeM01 ShreeM01 added team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Nov 10, 2022
@ShreeM01 ShreeM01 requested a review from coeuvre November 10, 2022 00:27
@meteorcloudy meteorcloudy enabled auto-merge (squash) November 10, 2022 09:15
@meteorcloudy
Copy link
Copy Markdown
Member

@coeuvre Please LGTM if this looks good to be cherry-picked for 6.0 ;)

Copy link
Copy Markdown
Member

@coeuvre coeuvre left a comment

Choose a reason for hiding this comment

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

Yes, it's safe to cherry-pick this into 6.0.

@meteorcloudy
Copy link
Copy Markdown
Member

@Yannic Can you rebase?

@Yannic
Copy link
Copy Markdown
Contributor Author

Yannic commented Nov 10, 2022

thanks! done

@meteorcloudy meteorcloudy merged commit f589512 into bazelbuild:release-6.0.0 Nov 10, 2022
@Yannic Yannic deleted the yannic-cp-128d833fee99f8a43bc4de82cbec752e4ce6fb47 branch November 10, 2022 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR is awaiting review from an assigned reviewer team-Remote-Exec Issues and PRs for the Execution (Remote) team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants