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 informational headers to gRPC requests sent for datastore #2230

Closed
dhermes opened this issue Aug 31, 2016 · 8 comments
Closed

Add informational headers to gRPC requests sent for datastore #2230

dhermes opened this issue Aug 31, 2016 · 8 comments
Assignees
Labels
api: datastore Issues related to the Datastore API. grpc priority: p2 Moderately-important priority. Fix may not be included in next release.

Comments

@dhermes
Copy link
Contributor

dhermes commented Aug 31, 2016

See googleapis/google-cloud-node#1542

We need to modify the headers being sent to:

headers = [
    ('authorization', 'Bearer ' + access_token),
    ('user-agent', self._user_agent),
    ('google-cloud-resource-prefix', 'projects/my-project-id'),
]

Though we may want to do this on a per-request basis. Probably not, since self._user_agent doesn't change after the MetadataPlugin object is created, even if the user agent on the original Client has changed.

@dhermes dhermes added api: datastore Issues related to the Datastore API. grpc labels Aug 31, 2016
@tseaver
Copy link
Contributor

tseaver commented Sep 6, 2016

#2221 is broader than this issue would suggest.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 9, 2016

@tseaver Focusing on the datastore case, this is a bit awkward even as-is. The Connection doesn't have access to the project, since the Client handles that.

But the Connection is essentially a dumb wrapper around _DatastoreAPIOverHttp / _DatastoreAPIOverGRPC combined with some helpers for constructing the protobufs to be sent off.

WDYT about removing the datastore.connection module all together?

@tseaver
Copy link
Contributor

tseaver commented Sep 9, 2016

The Connection class is still responsible for managing the low-leve REST bits: I guess you could gut those parts into the _DatastoreAPIOverHttp class, and have the client just hold an instance of it (or the gRPC variant). They still need somewhere to live, which makes trashing the module problematic.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 9, 2016

is still responsible for managing the low-leve REST bits

Not really

@tseaver
Copy link
Contributor

tseaver commented Sep 9, 2016

That example delgates to the connection's _build_api_url method, uses its USER_AGENT, an calls into its http attr to make the actual request. Those were the bits I was talking about moving into _DatastoreAPIOverHttp.

@dhermes
Copy link
Contributor Author

dhermes commented Sep 9, 2016

Gotcha! That was just "laziness" to get the implementation to fit. I've got a speculative PR in the works, coming soon. We can chat about that?

@lukesneeringer lukesneeringer added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Apr 19, 2017
@lukesneeringer
Copy link
Contributor

Hello,
One of the challenges of maintaining a large open source project is that sometimes, you can bite off more than you can chew. As the lead maintainer of google-cloud-python, I can definitely say that I have let the issues here pile up.

As part of trying to get things under control (as well as to empower us to provide better customer service in the future), I am declaring a "bankruptcy" of sorts on many of the old issues, especially those likely to have been addressed or made obsolete by more recent updates.

My goal is to close stale issues whose relevance or solution is no longer immediately evident, and which appear to be of lower importance. I believe in good faith that this is one of those issues, but I am scanning quickly and may occasionally be wrong. If this is an issue of high importance, please comment here and we will reconsider. If this is an issue whose solution is trivial, please consider providing a pull request.

Thank you!

@eddavisson
Copy link

@lukesneeringer, could we re-open this?

We are still interested in having this header added to gRPC requests:

google-cloud-resource-prefix: projects/my-project-id

(This functionality is already in place for Spanner and Firestore.)

This is may not be relevant for all APIs, so I think this is the right issue to re-open as opposed to #2221.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API. grpc priority: p2 Moderately-important priority. Fix may not be included in next release.
Projects
None yet
Development

No branches or pull requests

4 participants