-
Notifications
You must be signed in to change notification settings - Fork 69
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
chore: allow custom routing header metadata #2079
Conversation
)), | ||
) | ||
metadata = tuple(metadata) | ||
if all(m[0] != gapic_v1.routing_header.ROUTING_METADATA_KEY for m in metadata): |
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.
Please can you add comments to clarify the reason for the additional logic?
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.
I updated the comment above this block to make it more clear, let me know what you think
@parthea I'm a little unsure about the failing showcase tests. Looking at the method names, it seems to line up with the methods in _test_mixins. But I don't fully understand that file. Do I need to duplicate my new tests for each method in _test_mixins by hand? Why is that needed on top of the templates I already added to test_macros? |
Unrelated to this change, a customer reported a separate issue with routing for the async client. We may want to address this at the same time? #2091 |
Blocked on #2133. I will update this after those changes are merged |
From #2133 (comment):
|
Closing this because it is out of date after #2133 (and may no longer be needed) |
Avoid setting the routing metadata field (
x-goog-request-params
) if it already exists in the passed in metadata, to prevent duplicate routing headersFixes #2078