Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

feat: Add protobuf runtime version to x-goog-api-client header - #2368

Merged
parthea merged 2 commits into
mainfrom
add-protobuf-runtime
May 6, 2025
Merged

feat: Add protobuf runtime version to x-goog-api-client header#2368
parthea merged 2 commits into
mainfrom
add-protobuf-runtime

Conversation

@parthea

@partheaparthea commented Mar 19, 2025

Copy link
Copy Markdown
Contributor

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:

urlRequestHeaders:
X-Goog-Api-Client: "gl-python/3.9.16 rest/google-auth@2.40.0 gax/2.24.2 gapic/0.0.0"

After (with googleapis/python-api-core#812):

urlRequestHeaders:
X-Goog-Api-Client: "gl-python/3.9.16 rest/google-auth@2.40.0 gax/2.24.2 gapic/0.0.0 pb/6.30.2"

You can test the change by forcing installation of PR googleapis/python-api-core#812 using the change below

py3133partheniou@partheniou-vm-3:~/git/gapic-generator-python$ git diff noxfile.py
diff --git a/noxfile.py b/noxfile.py
index 117ca47c..899fdd4a 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -373,6 +373,10 @@ def showcase_library(
"--upgrade",
"google-auth[aiohttp]!=2.40.0",
)
+ session.install(
+ "--ignore-installed",
+ "google-api-core @ git+https://github.com/googleapis/python-api-core.git@add-protobuf-runtime-header",
+ )
else:
# The ads templates do not have constraints files.

and then run nox -s showcase_w_rest_async-3.9

@product-auto-labelproduct-auto-labelBot added the size: s Pull request size is small. label Mar 19, 2025
@parthea
partheaforce-pushed the add-protobuf-runtime branch from 8901a49 to fe73db3CompareMarch 19, 2025 21:22
@product-auto-labelproduct-auto-labelBot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Mar 19, 2025
@parthea
partheaforce-pushed the add-protobuf-runtime branch from fe73db3 to b59e0faCompareMarch 19, 2025 21:23
@partheaparthea closed this Mar 20, 2025
@partheaparthea changed the title for testing purposesfeat: Add protobuf runtime version to x-goog-api-client headerMay 6, 2025
@partheaparthea reopened this May 6, 2025
@parthea
parthea marked this pull request as ready for review May 6, 2025 15:05
@parthea
parthea requested a review from a teamMay 6, 2025 15:05
@parthea
partheaforce-pushed the add-protobuf-runtime branch from 5f62265 to 88efc0eCompareMay 6, 2025 18:49

@vchudnov-gvchudnov-g left a comment

Copy link
Copy Markdown
Contributor

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.

@parthea

Copy link
Copy Markdown
ContributorAuthor

Correct, the format string has a fixed order but it's not clear how problematic this is.

but it also requires these guard statements at the calling site

The guard statements are often necessary for backwards compatibility reasons for any new feature that we add to google-api-core

@parthea
parthea merged commit 9c05dbe into mainMay 6, 2025
@parthea
parthea deleted the add-protobuf-runtime branch May 6, 2025 22:59
@release-pleaserelease-pleaseBot mentioned this pull request May 6, 2025
@vchudnov-g

Copy link
Copy Markdown
Contributor

Yeah, ignore my comment about the order. I got confused.

But this is something that shouldn't need a guard clause, if api-core had been implemented differently. IMO, we should be able to set arbitrary key-value pairs in ClientInfo, without the need for the guard clause here. When emitting the header, ClientInfo would only look for the keys it knows about to create the format string. So older versions would not use the protobuf info, but as soon as people were using the newer version of api-core, that would become part of the header. This would make things more extensible, though admittedly it would be less type-safe, since a typo would cause the header token to be omitted.

Oh, well. Hindsight.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: mPull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@parthea@vchudnov-g