Skip to content
This repository has been archived by the owner on Mar 20, 2018. It is now read-only.

Remove oauth2client #176

Merged
merged 1 commit into from
Mar 27, 2017
Merged

Remove oauth2client #176

merged 1 commit into from
Mar 27, 2017

Conversation

theacodes
Copy link

Closes #155 and #175

@theacodes
Copy link
Author

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
Copy link
Contributor

lukesneeringer commented Mar 23, 2017

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 googleapis/google-cloud-python#1998 which is @dhermes current project this week.

@dhermes
Copy link
Contributor

dhermes commented Mar 23, 2017

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
Copy link
Author

How much work would this be?

Not much, once googleapis/google-auth-library-python#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
Copy link
Author

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
Copy link
Contributor

dhermes commented Mar 23, 2017

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
Copy link
Author

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
Copy link
Author

bump?

@lukesneeringer lukesneeringer merged commit 85892ef into master Mar 27, 2017
@theacodes
Copy link
Author

👏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants