Ensure namedSetOfFiles URIs specify blob type correctly #19044
Conversation
github-actions
bot
added
awaiting-review
tylerwilliams
changed the title
|
@coeuvre I think this is the last one |
| } | ||
|
|
||
| /** | ||
| * Collects metadata for {@code file}. Depending on the underlying filesystem used this method | ||
| * might do I/O. | ||
| */ | ||
| private PathMetadata readPathMetadata(Path path, LocalFile file) throws IOException { | ||
| DigestUtil digestUtil = new DigestUtil(xattrProvider, path.getFileSystem().getDigestFunction()); |
There was a problem hiding this comment.
I think we can just use path.getFileSystem().getDigestFunction() instead of creating a DigestUtil.
There was a problem hiding this comment.
DigestUtil exposes the DigestFunction.Value which is distinct from DigestHashFunction which is returned by the method you're suggesting.
So we're using DigestUtil to convert basically.
There was a problem hiding this comment.
Also -- sorry it wasn't clear, DigestUtil was already being created in this function (L217), i just moved it up so I can reference it :)
There was a problem hiding this comment.
Argh, yes. Sorry I didn't read the code carefully.
| Digest digest = metadata.getDigest(); | ||
| DigestFunction.Value digestFunction = metadata.getDigestFunction(); | ||
| String out; | ||
| if (isOldStyleDigestFunction(digestFunction)) { |
There was a problem hiding this comment.
Can we have a unit test for this?
|
Optional: we might also want to make similar changes to Http/Disk cache in a separate PR. |
tylerwilliams
force-pushed
the
blake3_namedsetoffiles
branch
from
2b83de5
to
e181273
Compare
|
Thanks! I probably need more time to import this because I need to figure out how to build blake3 internally for remote module so the test can be executed (previously, I skipped that). |
github-actions
bot
removed
the
awaiting-review
|
The changes in this PR have been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=last_rc. |
I noticed when testing the BLAKE3 digest function that uploaded files were being referenced incorrectly in the BES because they were missing the digest type. This CL fixes that.
Before:
After: