remote-apis
remote-apis copied to clipboard
bazelbuild
ActionCache: explicit delete operation?
The action cache currently does not have a delete operation. According to GCP, the RBE permissions include a delete permission. One could consider an update request without a result as a delete. However, this isn't clear from the documentation. I think I'd have a slight preference for having a separate delete operation. WDYT?
"Without a result" presumably meaning the default ActionResult instance? Hard to make a request otherwise.
Incidentally, can you refer to the documentation where this permission is enumerated here? I went looking for something similar recently, and I think we need the language associated with the permission to agree on action anyway.
I don't recall why RBE has an action 'delete' permission - most likely the permission exists but with no associated methods that leverage it. What is your use-case that needs Delete?
Whatever the reason, I have no specific objection to a Delete* method for actions, if you have a reason to need such a thing. But I also haven't encountered much call for it, and will note that RBE has no such need (or method) today*.
* Through the front door, anyways - we remove AC entries with garbage collection, for example, but that's an implementation detail.
Google's RBE permissions are documented here: https://cloud.google.com/iam/docs/permissions-reference (search for remotebuildexecution)
@werkt: the Java protobuf API does distinguish between unset protos and default protos, it only doesn't allow doing this for primitive types.
We may need an API to delete malformed, broken, or malicious action cache entries. Technically, overwriting an action cache entry may also be possible by setting skip_cache_lookup=true, but that incurs the cost of re-executing the action, and the service typically only writes an entry if the exit code is zero.
Conceptually I have no objection to an explicit delete API. We originally opted not to implement one at least in part because constructing the digest to delete is relatively difficult, but that was purely a pragmatic decision based on the client tooling available at the time.
I'd vote in favour of an explicit delete in preference to overloading UpdateActionResult. Agreed that this is a useful thing in practice.
We might want to consider extending ActionCacheUpdateCapabilities to indicate whether delete is enabled (or clarify that update_enabled covers both).
In concept I would agree. @ulfjack, would you mind bringing this back to the mailing list to ensure broader visibility of the question? If you already have a proposal for the DeleteActionResult API, maybe open a WIP PR?
