-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Adding ability to send version info header on HTTP requests. #3035
Conversation
Added an "extra headers" feature to enable this. I am not a fan of changing `Connection()` so haphazardly, but I hope to completely re-factor / destory `Connection()` in the near-term so I am less worried. This only adds the storage and datastore header info, for the purposes of a simple review. Once we agree on the approach, I can add support in the other subpackages.
@@ -61,6 +62,9 @@ | |||
|
|||
_DISABLE_GRPC = os.getenv(DISABLE_GRPC, False) | |||
_USE_GRPC = _HAVE_GRPC and not _DISABLE_GRPC | |||
_DATASTORE_DIST = get_distribution('google-cloud-datastore') | |||
_CLIENT_INFO = connection_module.CLIENT_INFO_TEMPLATE.format( | |||
_DATASTORE_DIST.version) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
Thanks @tseaver. Will hold off for @lukesneeringer review before merging |
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.
This looks fine to me. I am not clear how the where the handling for the gccl
segment is when something is using GAX. I assume that is in a different PR.
Will follow up with a PR for all other HTTP impls and then a separate PR getting this to work with GAPIC generated surfaces |
Adding ability to send version info header on HTTP requests.
Added an "extra headers" feature to enable this. I am not a fan
of changing
Connection()
so haphazardly, but I hope tocompletely re-factor / destory
Connection()
in the near-termso I am less worried.
This only adds the storage and datastore header info, for the
purposes of a simple review. Once we agree on the approach,
I can add support in the other subpackages.