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

Why google.auth.transport.secure_authorized_channel() needs a Request object? #239

Closed
fengli79 opened this issue Feb 8, 2018 · 14 comments
Closed
Assignees
Labels
🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@fengli79
Copy link

fengli79 commented Feb 8, 2018

When using google.auth.transport.secure_authorized_channel(), I have to specify a Request() object. Why it's required?

@theacodes
Copy link
Contributor

theacodes commented Feb 8, 2018

Because some credential types must make an HTTP request to refresh themselves.

@fengli79
Copy link
Author

fengli79 commented Feb 8, 2018

This doesn't explain why the request object need to be provided by the caller, and why it cannot be optional. For example, the secure_authorized_channel() may create one inside it when such a request is not given.

@theacodes
Copy link
Contributor

The request object has to be provided by the caller because google-auth is a low-level transport-agnostic library. It can not make the decision of which transport the calling code must use.

Higher-level clients, such as google-cloud-python, can make that decision and generally wrap secure_authorized_channel. See https://github.com/GoogleCloudPlatform/google-cloud-python/blob/d84d7f5ea180d9e4bf254de91920dc69249ade91/api_core/google/api_core/grpc_helpers.py#L114.

@fengli79
Copy link
Author

fengli79 commented Feb 8, 2018

Thanks for the response. However, in the above link, a Request object is always created when creating a secure gRPC channel. It sounds better to put the creation of the Request object inside secure_authorized_channel() and make the request argument optional.

@theacodes
Copy link
Contributor

It doesn't. See my previous comment:

google-auth is a low-level transport-agnostic library. It can not make the decision of which transport the calling code must use.

@lukesneeringer
Copy link

lukesneeringer commented Feb 8, 2018

Additionally, doing that would require a hard dependency on whichever request library (presumably requests) was chosen as the default, which a transport-agnostic library should avoid. We should not be bringing along non-trivial libraries that are actually only used downstream.

Also, why are we litigating this? The library is 1.0.

@theacodes
Copy link
Contributor

theacodes commented Feb 8, 2018

You can read more about how transports work here: https://google-auth.readthedocs.io/en/latest/user-guide.html#making-authenticated-requests

and more about our design considerations here: #1 and #15.

@fengli79
Copy link
Author

fengli79 commented Feb 8, 2018

Both secure_authorized_channel() and google.auth.transport.requests.Request are in the same lib. I don't see why secure_authorized_channel() cannot use it as the default when user don't want to provide one.

@lukesneeringer
Copy link

Because that module may not be importable (see my comment above).

@theacodes
Copy link
Contributor

Again, I implore you to review the resources in #239 (comment). This library has no dependencies on any specific HTTP library and support several options.

@fengli79
Copy link
Author

fengli79 commented Feb 8, 2018

It does, as pointed out by @lukesneeringer 's previous comment. It tries to import the requests package.
Yes, it needs to handle the case that requests module is not installed.
However when requests module is installed, it does import it and can use it as default http client. In that case, it doesn't make sense to force user to create a request object, they should be able to skip the request parameter when requests modules have been installed.
It's perfectly fine to raise an exception when requests module is not installed and the request argument are not provided.

@theacodes
Copy link
Contributor

theacodes commented Feb 8, 2018

If we haven't done a good job of explaining this, I apologize:

This library is low-level. It provides no defaults for transports. It can not and will not ever do so. It's a core design decision discussed in detail in #1 and #15 based on extraordinarily difficult lessons learned during maintenance of oauth2client.

There are multiple downstream users of this library and it's entirely possible that every transport option (urllib3, requests, and through extensions httplib2) is available and we can not just arbitrarily choose at this layer. The layer above this one determines the transport. For example, if google-api-python-client, google-cloud-storage, and google-cloud-datastore are all installed and used in an application, they will all use google-auth but they will all use different transports (httplib2, requests, and grpc+requests respectively).

To put this even more plainly: Whatever your use case is, if you're asking for this you are interacting with the wrong level. The level you are likely after is google.api_core where we can and do make the explicit decision to use requests.

@fengli79
Copy link
Author

fengli79 commented Feb 9, 2018

Thanks a lot for the explanation, really appreciate to share the stories back that decision, however I'm not convinced. Those client libraries can always explicitly specify the http client.

Here's my story, if someone use a gRPC client generated from the protos published in googleapis/googleapis, even when they installed the requests module, they have to create boilerplate code to create a request object to create a secure channel, like:
credentials, _ = google.auth.default() http_request = Request() channel = secure_authorized_channel(credentials, http_request, 'pubsub.googleapis.com')

Note, they cannot use google.api_core as they use gRPC directly. When requests module get installed, they should be able to simply do this:
credentials, _ = google.auth.default() channel = secure_authorized_channel(credentials, 'datastore.googleapis.com')

@theacodes
Copy link
Contributor

theacodes commented Feb 9, 2018

I'm not convinced

That's okay, as your use case is not an intended target for this library.

hey have to create boilerplate code to create a request object to create a secure channel

It's not that much boilerplate, honestly.

Note, they cannot use google.api_core as they use gRPC directly.

Yes they can, api_core has a public interface and can be used all by itself, e.g.:

from google.api_core import grpc_helpers

channel = grpc_helpers.create_channel('datastore.googleapis.com')

@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

4 participants