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: add support for bazel qualifiers #7412

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

sluongng
Copy link
Contributor

In addition to the checksum.sri qualifier from remote api spec,
Bazel includes it's own set of qualifiers to each FetchBlob requests.

https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java;l=76-85;drc=618c0abbfe518f4e29de523a2e63ca9179050e94

This change adds initial support for the http_header qualifier as well
as acknoledge the existence of bazel.canonical_id header (without
actually using it). Relevant tests were also added to demonstrate the
current behavior.

In a future patch, we may start rejecting unknown qualifiers as
recommended by the Remote Asset API spec and thus, flip the assertions
in the test. See bazelbuild/remote-apis#301
for more info.

Another change to consider in the future is support for custom
url-specific header credentials. This is being proposed in Bazel via
bazelbuild/bazel#23578.

Copy link
Member

@bduffany bduffany left a comment

Choose a reason for hiding this comment

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

nice 👍

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
In addition to the checksum.sri qualifier from remote api spec,
Bazel includes it's own set of qualifiers to each FetchBlob requests.

https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java;l=76-85;drc=618c0abbfe518f4e29de523a2e63ca9179050e94

This change adds initial support for the http_header qualifier as well
as acknoledge the existence of bazel.canonical_id header (without
actually using it). Relevant tests were also added to demonstrate the
current behavior.

In a future patch, we may start rejecting unknown qualifiers as
recommended by the Remote Asset API spec and thus, flip the assertions
in the test. See bazelbuild/remote-apis#301
for more info.

Another change to consider in the future is support for custom
url-specific header credentials. This is being proposed in Bazel via
bazelbuild/bazel#23578.
@sluongng sluongng force-pushed the sluongng/fetch-server-bazel-qual branch from 5d2d9a5 to 66ca793 Compare September 11, 2024 13:57
@sluongng sluongng merged commit 49939ef into master Sep 13, 2024
15 checks passed
@sluongng sluongng deleted the sluongng/fetch-server-bazel-qual branch September 13, 2024 10:28
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