-
Notifications
You must be signed in to change notification settings - Fork 70
feat: Add protobuf runtime version to x-goog-api-client
header
#2368
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
Conversation
8901a49
to
fe73db3
Compare
fe73db3
to
b59e0fa
Compare
x-goog-api-client
header
5f62265
to
88efc0e
Compare
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.
Note that the implementation in https://github.com/googleapis/python-api-core/blob/118bd96f3907234351972409834ab5309cdfcee4/google/api_core/client_info.py#L114 is problematic. Not only does it depend on us constructing the format string in the same order as the __dict__
entries, but it also requires these guard statements at the calling site. We can/should refactor that code to make this unnecessary going forward, except we'll still need to account for people using api_core
from before the refactor.....we'll be stuck with these guard clauses ~indefinitely now.
Correct, the format string has a fixed order but it's not clear how problematic this is.
The guard statements are often necessary for backwards compatibility reasons for any new feature that we add to |
Yeah, ignore my comment about the order. I got confused. But this is something that shouldn't need a guard clause, if Oh, well. Hindsight. |
See https://cloud.google.com/apis/docs/system-parameters#definitions
Googlers see b/401039328
This change is built on top of #2386 which fixes a presubmit which is failing at main.
Before:
After (with googleapis/python-api-core#812):
You can test the change by forcing installation of PR googleapis/python-api-core#812 using the change below
and then run
nox -s showcase_w_rest_async-3.9