feat(common): support injecting custom headers #13829
Conversation
| * An option to inject custom headers into the request. | ||
| * | ||
| * For REST endpoints, these headers are added to the HTTP headers. For gRPC | ||
| * endpoints, these headers are added to the `grpc::ClientContext` metadata. |
There was a problem hiding this comment.
add: @ingroup options which I am noticing I forgot to do for EnableServerRetriesOption.
| if (options.has<google::cloud::CustomHeadersOption>()) { | ||
| for (auto const& h : options.get<google::cloud::CustomHeadersOption>()) { |
There was a problem hiding this comment.
Drop the options.has<> conditional?
Also, s/google::cloud::CustomHeadersOption/CustomHeadersOption/
| if (options.has<google::cloud::CustomHeadersOption>()) { | ||
| for (auto const& h : options.get<google::cloud::CustomHeadersOption>()) { |
| @@ -406,6 +406,11 @@ void $metadata_class_name$::SetMetadata(grpc::ClientContext& context, | |||
| } | |||
| auto const& authority = options.get<AuthorityOption>(); | |||
| if (!authority.empty()) context.set_authority(authority); | |||
| if (options.has<google::cloud::CustomHeadersOption>()) { | |||
There was a problem hiding this comment.
Optional nit: It looks like we're a little inconsistent with our qualifying. Options a few lines up, for example, is mentioned without google::cloud::.
| @@ -406,6 +406,11 @@ void $metadata_class_name$::SetMetadata(grpc::ClientContext& context, | |||
| } | |||
| auto const& authority = options.get<AuthorityOption>(); | |||
| if (!authority.empty()) context.set_authority(authority); | |||
| if (options.has<google::cloud::CustomHeadersOption>()) { | |||
There was a problem hiding this comment.
Optional nit: You could do without the has<T>() test (and hence the double lookup) in this case, at the cost of a once-only allocation of an empty map.
scotthart
force-pushed
the
rest_custom_http_headers
branch
from
968fdd8
to
50c15c9
Compare
There was a problem hiding this comment.
Reviewable status: 0 of 427 files reviewed, 5 unresolved discussions (waiting on @dbolduc and @devbww)
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)
add:
@ingroup optionswhich I am noticing I forgot to do forEnableServerRetriesOption.
Added for all options in this file missing the tag.
a discussion (no related file):
Previously, devbww (Bradley White) wrote…
(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)
Optional nit: It looks like we're a little inconsistent with our qualifying.
Optionsa few lines up, for example, is mentioned withoutgoogle::cloud::.
Made qualification consistent across all options in the function.
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)
Drop the
options.has<>conditional?Also, s/google::cloud::CustomHeadersOption/CustomHeadersOption/
Done.
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)
ditto
Done.
a discussion (no related file):
Previously, devbww (Bradley White) wrote…
(Reviewable was unable to map this GitHub inline comment thread to the right spot — sorry!)
Optional nit: You could do without the
has<T>()test (and hence the double lookup) in this case, at the cost of a once-only allocation of an empty map.
Done.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #13829 +/- ##
==========================================
- Coverage 93.64% 93.04% -0.61%
==========================================
Files 2258 2174 -84
Lines 195012 184802 -10210
==========================================
- Hits 182620 171945 -10675
- Misses 12392 12857 +465 ☔ View full report in Codecov by Sentry. |
Adds support for adding custom headers for both gRPC and REST.
This change is