Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Conversation

nathanielmanistaatgoogle
Copy link
Contributor

This is #399 with tests/test__pycrypto_crypt.py changed to use unittest2.

@dhermes
Copy link
Contributor

dhermes commented Feb 5, 2016

LGTM, though I meant to update the s to json_str change actually be json_data (based on #396 or #397)

@nathanielmanistaatgoogle
Copy link
Contributor Author

I can change json_str to json_data.

Any idea why the tests haven't yet kicked off for this?

@googlebot
Copy link
Collaborator

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@nathanielmanistaatgoogle
Copy link
Contributor Author

@googlebot you're being silly and failing to recognize an authored-by-@dhermes-committed-by-@nathanielmanistaatgoogle commit as legitimate to merge.

This completes the consolidation of the two service
account credentials implementations.

In the process, also adding test coverage for some untested
code paths within the crypto helpers.
@nathanielmanistaatgoogle nathanielmanistaatgoogle force-pushed the consolidate-service-accounts-v4 branch from 8adcb63 to dcd20c9 Compare February 5, 2016 15:27
@dhermes
Copy link
Contributor

dhermes commented Feb 5, 2016

  1. ISTM the google org gets rate limited (also having 8 Travis envs probably doesn't help)
  2. I've had the CLA check fail like this before. I think it's a corner case the server isn't implemented to handle?

@nathanielmanistaatgoogle
Copy link
Contributor Author

Changed json_str to json_data in oauth2client/client.py; please take another quick look.

Any way to poke Travis to make it notice that a different commit has been pushed? I cancelled the test run of 81da3192423237b6.

@dhermes
Copy link
Contributor

dhermes commented Feb 5, 2016

You've gotta figure out what the rate limit is. Need to check with whoever owns the org

@dhermes
Copy link
Contributor

dhermes commented Feb 5, 2016

Still LGTM

nathanielmanistaatgoogle added a commit that referenced this pull request Feb 5, 2016
…vice-accounts-v4

Removed SignedJwtAssertionCredentials.
@nathanielmanistaatgoogle nathanielmanistaatgoogle merged commit 5db0a8b into googleapis:master Feb 5, 2016
@nathanielmanistaatgoogle nathanielmanistaatgoogle deleted the consolidate-service-accounts-v4 branch February 5, 2016 15:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants