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

bes: Allow additional headers to be passed to the server. #13812

Closed
wants to merge 1 commit into from

Conversation

benjaminp
Copy link
Collaborator

There is already a --remote_header option that passes extra headers to the remote cache and executor. This revision allows extra headers to be passed along to a BES server with the --bes_header option.

RELNOTES[NEW]: Add --bes_header flag to pass extra headers to the BES server.

@google-cla google-cla bot added the cla: yes label Aug 6, 2021
@benjaminp benjaminp force-pushed the bes-headers branch 2 times, most recently from 7a6d5a9 to 5bc2068 Compare August 6, 2021 23:44
stub = callCredentials != null ? stub.withCallCredentials(callCredentials) : stub;
stub = interceptor != null ? stub.withInterceptors(interceptor) : stub;
return stub;
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if both callCredentials and interceptor are present? Is that possible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The stub should be wrapped by with call-credentials and then the header interceptor.

@meisterT meisterT requested a review from michaeledgar August 12, 2021 10:23
Copy link
Contributor

@michaeledgar michaeledgar left a comment

Choose a reason for hiding this comment

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

Thanks for sending this pull request - the functionality makes sense and looks good!

I just have a couple of cleanups to suggest before merging.

There is already a --remote_header option that passes extra headers to the remote cache and executor. This revision allows extra headers to be passed along to a BES server with the --bes_header option.

RELNOTES[NEW]: Add `--bes_header` flag to pass extra headers to the BES server.
@bazel-io bazel-io closed this in ef42d13 Aug 14, 2021
@benjaminp benjaminp deleted the bes-headers branch August 14, 2021 19:43
bazel-io pushed a commit that referenced this pull request Aug 17, 2021
With this refactoring, the `BuildEventServiceGrpcClient` is not concerned with
interpreting command-line options and can operate purely on gRPC domain objects.
The `BazelBuildEventServiceModule` is responsible for preparing the domain objects
from the command-line options.

This refactoring was requested but skipped during PR #13812 because the
BazelBuildEventServiceModuleTest was not yet open-sourced. Now that the test is
open-sourced we can more easily separate (and test) these concerns.

PiperOrigin-RevId: 391347857
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants