Clarify UpdateActionResult requirements #351
Conversation
As discussed at the 2025-10-14 working group meeting.
tjgq
requested review from
EdSchouten,
EricBurnett,
bwkimmel,
fmeum,
sluongng,
sstriker and
ulfjack
as code owners
| // The client SHOULD also upload other blobs transitively referenced by the | ||
| // [Action][build.bazel.remote.execution.v2.Action] message into the | ||
| // `ContentAddressableStorage`. The server MAY return `FAILED_PRECONDITION` | ||
| // if this condition has not been met. |
There was a problem hiding this comment.
Just curious- why is this recommended?
If the this call is made by a remote execution worker uploading build results then all the CAS blobs required by the Action must already be in the cache. And if the caller is a regular client just using remote caching and not remote execution, then the Action is not technically required to be in the CAS.
There was a problem hiding this comment.
Perhaps this should read "The client is not required to" instead of "The client SHOULD"? The discussion happened a while ago, but if I recall correctly, the intent was precisely to document that clients are free not to meet the precondition, and servers are not required to enforce it. (If one of the other maintainers remembers it differently, please chime in.)
There was a problem hiding this comment.
The less strict version sounds good to me.
There was a problem hiding this comment.
The less strict version sounds good to me.
As discussed at the 2025-10-14 working group meeting.