Add compressed writes to cas.go. #240
Conversation
go/pkg/client/bytestream.go
Outdated
| // Notice that the digest in the chunker might be misleading. | ||
| // Specifically, for compressed blob uploads, the resource | ||
| // name should include the uncompressed digest - while chunker | ||
| // should be including the compressed digest. |
There was a problem hiding this comment.
I thought that you removed the Digest() function from Chunker altogether, instead relying on UE.digest
There was a problem hiding this comment.
Yes - this is just an artifact of rebasing :)
go/pkg/client/tree.go
Outdated
| @@ -65,7 +65,7 @@ func shouldIgnore(inp string, t command.InputType, excl []*command.InputExclusio | |||
|
|
|||
| // loadFiles reads all files specified by the given InputSpec (descending into subdirectories | |||
| // recursively), and loads their contents into the provided map. | |||
| func loadFiles(execRoot string, excl []*command.InputExclusion, path string, fs map[string]*fileSysNode, cache filemetadata.Cache) error { | |||
| func (c *Client) loadFiles(execRoot string, excl []*command.InputExclusion, path string, fs map[string]*fileSysNode, cache filemetadata.Cache) error { | |||
There was a problem hiding this comment.
Where are you using the client here?
There was a problem hiding this comment.
I shouldn't - this was a bad rebasing.
rubensf
force-pushed
the
c-write-cup
branch
4 times, most recently
from
8256f1e to
a3b47c1
Compare
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
google-cla
bot
added
cla: no
|
@googlebot I fixed it. |
google-cla
bot
added
cla: yes
rubensf
force-pushed
the
c-read-cup
branch
2 times, most recently
from
9f91be9 to
abc1ab3
Compare
This follows the current tentative API being worked on in bazelbuild/remote-apis#168. While there's technically room for it to change, it has reached a somewhat stable point worth implementing.
| @@ -1377,3 +1387,14 @@ func (c *Client) DownloadFiles(ctx context.Context, execRoot string, outputs map | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| func (c *Client) shouldCompress(sizeBytes int64) bool { | |||
There was a problem hiding this comment.
nit: If the function is not reused, maybe inline?
There was a problem hiding this comment.
I'll be reusing it on read compression :)
| for _, blob := range tc.input { | ||
| input = append(input, uploadinfo.EntryFromBlob(blob)) | ||
| } | ||
| for _, cmp := range []client.CompressedBytestreamThreshold{-1, 0} { |
There was a problem hiding this comment.
Note to self: add a []client.Opt field to tests and expand the tests to contain the cartesian product of these options in a loop before running them. The indentation is getting out of hand...
(I'll clean this up separately)
There was a problem hiding this comment.
sgtm - if the indentation is the only problem you can also move the inner block into a separate function. Too late for it right now, but it is useful whereas in it preserves the original blame!
This follows the current tentative API. This PR potentially replaces #232