-
Notifications
You must be signed in to change notification settings - Fork 107
fix(lro): Add Operation name to headers in {Get,List}Operation requests [gax-java] #1281
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be tested?
Codecov Report
@@ Coverage Diff @@
## master #1281 +/- ##
============================================
+ Coverage 79.19% 79.34% +0.15%
Complexity 1235 1235
============================================
Files 209 209
Lines 5378 5398 +20
Branches 454 454
============================================
+ Hits 4259 4283 +24
+ Misses 939 936 -3
+ Partials 180 179 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if we can confirm that this will not break the other LRO operations which do not need routing.
@@ -149,11 +152,29 @@ protected GrpcOperationsStub( | |||
GrpcCallSettings<GetOperationRequest, Operation> getOperationTransportSettings = | |||
GrpcCallSettings.<GetOperationRequest, Operation>newBuilder() | |||
.setMethodDescriptor(getOperationMethodDescriptor) | |||
.setParamsExtractor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add it to all methods, not only to "get" ones (for completeness).
Not without breaking the layers of abstraction between OperationsClient and GrpcOperationsStub, as well as that between the LRO client and the rest of gax-java. |
Tested with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Similar to Python's issue. See internal issue 173104871 for more details.
Some APIs require routing headers (location info in
x-goog-request-params
) for successful requests (see also https://google.aip.dev/client-libraries/4222). These headers are added to normal requests to the API but are missing in calls to the Operations API. This results in requests failing with permission denied errors.