bazelbuild / bazelbuild/remote-apis

Asset API: Use of Directory instead of Tree for PushDirectory

Open
#165 2 comments 2 reactions 1 assignee Claimed by @sstriker View on GitHub
Dominant language
Go
Stars
445
Forks
141
PR merge metrics
No merged PRs in 30d

Description

The use of the [Directory](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.proto#L697-L710) instead of [Tree](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.proto#L1075-L1084) for [Push](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/asset/v1/remote_asset.proto#L409-L441)/[Fetch](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/asset/v1/remote_asset.proto#L287-L315) Directory leads to additional overhead on each Fetch which could instead be performed once by the client when pushing the Directories to the CAS, or once by the [Fetch Service](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/asset/v1/remote_asset.proto#L86-L150) when performing an [optional remote fetch](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/asset/v1/remote_asset.proto#L90-L91).

This overhead is made clear when attempting to ensure that the full directory Tree under the [root_directory_digest](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/asset/v1/remote_asset.proto#L314) is present with the CAS before responding to a [FetchDirectory](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/asset/v1/remote_asset.proto#L147-L150) request.

Whilst verifying the existence of these files is not a requirement within the spec, I have encountered issues with clients falling over following a Fetch request which returned a digest to an item which isn't in the CAS. Is this something clients should handle, or servers should [ensure the digests within ActionResults](https://github.com/bazelbuild/remote-apis/blob/master/build/bazel/remote/execution/v2/remote_execution.proto#L141-L146) exist within the CAS.

It appears that a similar change occurred during the early days of the RE spec. https://github.com/bazelbuild/remote-apis/commit/f85ddf0bb84a0b606b72d793f158e48862657455#diff-23d388eca6738e626128af49c6c8f0e5R745-R750

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.