Skip to content
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

Add client_info to constructors for manual clients #7825

Closed
14 tasks done
tswast opened this issue Apr 30, 2019 · 7 comments
Closed
14 tasks done

Add client_info to constructors for manual clients #7825

tswast opened this issue Apr 30, 2019 · 7 comments
Assignees
Labels
api: bigquery Issues related to the BigQuery API. api: bigtable Issues related to the Bigtable API. api: clouderrorreporting Issues related to the Error Reporting API. api: cloudresourcemanager Issues related to the Resource Manager API. api: cloudtrace Issues related to the Cloud Trace API. api: datastore Issues related to the Datastore API. api: dns Issues related to the Cloud DNS API. api: firestore Issues related to the Firestore API. api: logging Issues related to the Cloud Logging API. api: pubsub Issues related to the Pub/Sub API. api: runtimeconfig Issues related to the Cloud Runtime Config API API. api: spanner Issues related to the Spanner API. api: storage Issues related to the Cloud Storage API. api: translation Issues related to the Cloud Translation API API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@tswast
Copy link
Contributor

tswast commented Apr 30, 2019

The client_info constructor argument is the standard argument for changing user agent headers in GAPIC clients. Adding a user_agent prefix identifying an app or tool is required for many partners who use Google APIs.

Handwritten clients should provide a similar client_info argument to the client constructor. See how this was done in BigQuery at #7806.

Clients that need client_info:

I've heard requests for this feature in the Storage client, so I'd prioritize that one above the others, but we do need this feature across all clients.

@tswast tswast added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: datastore Issues related to the Datastore API. api: bigquery Issues related to the BigQuery API. api: dns Issues related to the Cloud DNS API. api: bigtable Issues related to the Bigtable API. api: clouderrorreporting Issues related to the Error Reporting API. api: firestore Issues related to the Firestore API. api: cloudresourcemanager Issues related to the Resource Manager API. api: cloudtrace Issues related to the Cloud Trace API. api: logging Issues related to the Cloud Logging API. api: pubsub Issues related to the Pub/Sub API. api: runtimeconfig Issues related to the Cloud Runtime Config API API. api: spanner Issues related to the Spanner API. api: storage Issues related to the Cloud Storage API. api: translation Issues related to the Cloud Translation API API. labels Apr 30, 2019
@tseaver
Copy link
Contributor

tseaver commented May 1, 2019

My current plan:

@tseaver
Copy link
Contributor

tseaver commented May 2, 2019

@tswast One thing I've noticed while deprecating the _EXTRA_HEADERS variable: you bend over backwards to allow self._EXTRA_HEADERS["foo"] = "bar". I think I'd rather avoid supporting that, and instead just allow assignment to it. WDYT?

@tswast
Copy link
Contributor Author

tswast commented May 2, 2019

I think I'd rather avoid supporting that, and instead just allow assignment to it. WDYT?

Seems reasonable, since we're making a new property name now. I bent over backwards because we had some tests that did key assignment and I wanted to maintain compatibility.

@tseaver
Copy link
Contributor

tseaver commented May 3, 2019

@tswast I just realized that since we've hoisted the logic for adding the X-Goog-API-Client header into the base class, we can avoid jamming it into the extra_headers return value, which makes allowing assignment into it simple.

@tseaver
Copy link
Contributor

tseaver commented May 7, 2019

@tswast I think we should leave this feature out of the firestore_v1beta1 client object: the Firestore team really doesn't want to encourage new users for that version of the API.

@tswast
Copy link
Contributor Author

tswast commented May 8, 2019

Thanks @tseaver ! The REST clients LGTM. A few questions on the gapic clients.

@tswast
Copy link
Contributor Author

tswast commented May 8, 2019

🎉 Thanks! 🎉

Are we following-up with a bunch of releases next (or at least for api_core)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the BigQuery API. api: bigtable Issues related to the Bigtable API. api: clouderrorreporting Issues related to the Error Reporting API. api: cloudresourcemanager Issues related to the Resource Manager API. api: cloudtrace Issues related to the Cloud Trace API. api: datastore Issues related to the Datastore API. api: dns Issues related to the Cloud DNS API. api: firestore Issues related to the Firestore API. api: logging Issues related to the Cloud Logging API. api: pubsub Issues related to the Pub/Sub API. api: runtimeconfig Issues related to the Cloud Runtime Config API API. api: spanner Issues related to the Spanner API. api: storage Issues related to the Cloud Storage API. api: translation Issues related to the Cloud Translation API API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants