Add support for google-auth #319
Conversation
googlebot
added
the
cla: yes
|
FYI @dhermes - I don't know if you care to look at this. |
| else: | ||
| raise EnvironmentError( | ||
| 'No authentication library is available. Please install either ' | ||
| 'oauth2client or google-auth.') |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@jonparrott I would like @landrito and @lukesneeringer (long-time Pythonista, first-time member of our team) to review this PR. I just gave the latter write access to this repo, and am waiting for him to accept. |
|
@bjwatson sounds good. No rush. @lukesneeringer @landrito - this is a very old codebase with a lot of problems. Keep in mind that I tried to be "minimally invasive" here. Feel free to set up some time if you want to talk about context/purpose. |
|
Gotcha! I'll take a stab at the review and get back to you if I need some clarification. |
|
Travis should now be passing. Note that this is the change the finally forced us to stop supporting Python 2.6 in this library. |
|
We'd already agreed to drop 2.6 in #203. |
|
@landrito @lukesneeringer friendly ping |
|
@jonparrott Sorry, getting used to actually watching GitHub notifications. Reviewing now. |
|
No worries. Thanks. :)
…On Thu, Jan 5, 2017 at 9:57 AM Luke Sneeringer ***@***.***> wrote:
@jonparrott <https://github.com/jonparrott> Sorry, getting used to
actually watching GitHub notifications. Reviewing now.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#319 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPUc0O3HGpsXHeEZbsaaNFJrgtEnbP8ks5rPS8NgaJpZM4LSV0u>
.
|
There was a problem hiding this comment.
The only major feedback I have is that you will probably end up with weird cases/errors in cases where HAS_GOOGLE_AUTH is true but credentials get passed in without being properly instantiated as a google.auth.credentials.Credentials object. A trap I fell into last week was trying to specify credentials as a dictionary (this was with oauth2client), which raised an exception in a confusing way. I found myself wishing that it had just instantiated the appropriate object for me. I still have that thought reading through this.
That said, this looks good, and will not make any existing, working situation worse that I can see. Ship it.
Good catch. Done. |
Arguments `http` and `credentials` are mutually exclusive as of googleapis/google-api-python-client#319. This makes the gcloud integration tests pass for me. (They have been disabled on Travis for long because of another reason, still unfixed I guess, #1931.)
DictionaryStorage implements an optionally-locked storage over a dictionary-like object.
Added dictionary storage for googleapis#319.
This also obviates the need to pass credentials into
discovery.buildordiscovery.build_from_documentif you're using ADC. This library can now automatically acquire those.