Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch_server: support digest function and blake3 #6382

Merged
merged 3 commits into from
Apr 23, 2024

Conversation

sluongng
Copy link
Contributor

Implement support for digest function in FetchBlobRequest as well as
usage of Blake3 as a digest func.

@sluongng sluongng marked this pull request as ready for review April 16, 2024 18:02
@sluongng sluongng force-pushed the sluongng/fetch-server-blake3 branch from 940ab9f to 93c847f Compare April 17, 2024 12:44
server/remote_asset/fetch_server/fetch_server.go Outdated Show resolved Hide resolved
server/remote_asset/fetch_server/fetch_server.go Outdated Show resolved Hide resolved
server/remote_asset/fetch_server/fetch_server.go Outdated Show resolved Hide resolved
server/remote_asset/fetch_server/fetch_server.go Outdated Show resolved Hide resolved
SizeBytes: 1,
}
cacheRN := digest.NewResourceName(blobDigest, instanceName, rspb.CacheType_CAS, checksumFunc)
log.CtxInfof(ctx, "Looking up %s in cache", blobDigest.Hash)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdyt about removing these log.Info lines in the non-error cases? I think they might be a tad excessive

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion.

I will switch these to Debug level instead of removing them.
These could be fairly useful for development purposes.

server/remote_asset/fetch_server/fetch_server.go Outdated Show resolved Hide resolved
server/remote_asset/fetch_server/fetch_server.go Outdated Show resolved Hide resolved
sluongng and others added 3 commits April 23, 2024 07:12
Implement support for digest function in FetchBlobRequest as well as
usage of Blake3 as a digest func.
Switch happy path log to Debug
@sluongng sluongng force-pushed the sluongng/fetch-server-blake3 branch from aff4135 to 55556ae Compare April 23, 2024 05:15
@sluongng sluongng merged commit d532563 into master Apr 23, 2024
16 of 19 checks passed
@sluongng sluongng deleted the sluongng/fetch-server-blake3 branch April 23, 2024 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants