search

googleapis / gax-python

Google API Extensions for Python
http://gax-python.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
25 stars 13 forks source link

Remove oauth2client #176

Closed theacodes closed 7 years ago

theacodes commented 7 years ago

Closes #155 and #175

theacodes commented 7 years ago

Open question before merging: right now, gax does not directly depend on any of the transports needed for google-auth. It will check for requests, then httplib2, then give up code.

We have options:

  1. We can leave this as-is and I think everything will be okay.
    • google-cloud-python will include httplib2 and google-auth-httplib2 for now. google-cloud-python also never hits this code path.
    • We can make the gapic- packages depend on httplib2 and google-auth-httplib2 or requests, that ensures they always have a transport.
    • Alternatively we can make all gapic- packages upgrade to JWT credentials which do not need a transport. We should do this at some point anyway.
  2. We can add a dependency here on one of the two transports.
lukesneeringer commented 7 years ago

Alternatively we can make all gapic- packages upgrade to JWT credentials which do not need a transport. We should do this at some point anyway.

How much work would this be?

We can add a dependency here on one of the two transports.

How does that work in principle? If I were to add a dependency on urllib3 somewhere, would urllib3 objects get picked up instead of httplib2? (Summoning @dhermes).

Note that this is related to GoogleCloudPlatform/google-cloud-python#1998 which is @dhermes current project this week.

dhermes commented 7 years ago

Alternatively we can make all gapic- packages upgrade to JWT credentials which do not need a transport. We should do this at some point anyway.

Indeed, this should be the place we end up. Fewer round trips are better for everyone.

theacodes commented 7 years ago

How much work would this be?

Not much, once https://github.com/GoogleCloudPlatform/google-auth-library-python/issues/136 is done it's just a matter of credentials = jwt.OnDemandCredentials.from_signing_credentials(default_credentials). Although now I realize I've mislead you- even with that we still need a transport in the case that ADC returns user credentials (as is the case with gcloud) or GCE credentials.

How does that work in principle? If I were to add a dependency on urllib3 somewhere, would urllib3 objects get picked up instead of httplib2?

We'd have to add a little code to use urllib3 (currently this code just uses requests or httplib2), but yeah, if someone directly constructed a gapic client without a channel or credentials, then it would use whatever transport we depend on here. This is independent of google-cloud-python because afaik google-cloud-python constructs a channel before constructing a gapic client which bypasses all of the ADC logic here.

That is to say, in practice we can go ahead and make this library use urllib3 for the transport and it won't break google-cloud-python.

theacodes commented 7 years ago

Oh, another thing for jwt-only credentials: we should verify independently that every API supports this. I've only ever tested with pubsub and bigtable.

dhermes commented 7 years ago

Although now I realize I've mislead you- even with that we still need a transport in the case that ADC returns user credentials (as is the case with gcloud) or GCE credentials.

We can always just tell those credentials to go away. Also, for GCE creds, won't there be a local bytes signing API at some point?

theacodes commented 7 years ago

We can always just tell those credentials to go away.

We can't, we need to support every case that ADC supports.

Also, for GCE creds, won't there be a local bytes signing API at some point?

Yes but no timeline on that. :)

theacodes commented 7 years ago

bump?

theacodes commented 7 years ago

👏