Issue 4956059: client_secrets.json file format support

Issue 4956059: client_secrets.json file format support (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
13 years ago by jcgregorio_google

Modified:
12 years, 11 months ago

Reviewers:
aiuto, jcgregorio_google, Ali Afshar

CC:
google-api-python-client_googlegroups.com

Visibility:
Public.

Patch Set 2 : Add missing clientsecrets.py and client_secrets.json files. #

Total comments: 12

Patch Set 3 : Respond to review comments. #

Created: 12 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+443 lines, -27 lines)			Patch
M	oauth2client/appengine.py	View	1 2	7 chunks	+99 lines, -1 line	0 comments	Download
M	oauth2client/client.py	View	1 2	4 chunks	+75 lines, -1 line	0 comments	Download
A	oauth2client/clientsecrets.py	View	1 2	1 chunk	+113 lines, -0 lines	0 comments	Download
A	samples/appengine_with_decorator2/client_secrets.json	View	1	1 chunk	+9 lines, -0 lines	0 comments	Download
M	samples/appengine_with_decorator2/main.py	View	1	2 chunks	+31 lines, -11 lines	0 comments	Download
M	samples/buzz/buzz.py	View	1	1 chunk	+27 lines, -14 lines	0 comments	Download
A	samples/buzz/client_secrets.json	View	1	1 chunk	+9 lines, -0 lines	0 comments	Download
A	tests/test_oauth2client_clientsecrets.py	View	1 2	1 chunk	+80 lines, -0 lines	0 comments	Download

Messages

Total messages: 8

Expand All Messages | Collapse All Messages

jcgregorio_google

OK, looks like client_secrets.json support is a go, so please review now.

12 years, 11 months ago (2011-10-05 15:55:52 UTC) #2

Ali Afshar

Hi Joe, I don't see clientsecrets.py anywhere, am I missing something?

12 years, 11 months ago (2011-10-06 13:52:49 UTC) #4

jcgregorio_google

On 2011/10/06 13:52:49, Ali Afshar wrote: > Hi Joe, > > I don't see clientsecrets.py ...

12 years, 11 months ago (2011-10-06 15:28:17 UTC) #5

Ali Afshar

LGTM, nice patch. http://codereview.appspot.com/4956059/diff/7001/oauth2client/appengine.py File oauth2client/appengine.py (right): http://codereview.appspot.com/4956059/diff/7001/oauth2client/appengine.py#newcode56 oauth2client/appengine.py:56: class InvalidClientSecretsError(Exception): pass Add a docstring, ...

12 years, 11 months ago (2011-10-06 23:53:43 UTC) #6

jcgregorio_google

http://codereview.appspot.com/4956059/diff/7001/oauth2client/appengine.py File oauth2client/appengine.py (right): http://codereview.appspot.com/4956059/diff/7001/oauth2client/appengine.py#newcode56 oauth2client/appengine.py:56: class InvalidClientSecretsError(Exception): pass On 2011/10/06 23:53:43, Ali Afshar wrote: ...

12 years, 11 months ago (2011-10-07 13:09:03 UTC) #7

Ali Afshar

12 years, 11 months ago (2011-10-07 13:28:20 UTC) #8

LGTM

On 2011/10/07 13:09:03, jcgregorio_google wrote:
> http://codereview.appspot.com/4956059/diff/7001/oauth2client/appengine.py
> File oauth2client/appengine.py (right):
> 
>
http://codereview.appspot.com/4956059/diff/7001/oauth2client/appengine.py#new...
> oauth2client/appengine.py:56: class InvalidClientSecretsError(Exception): pass
> On 2011/10/06 23:53:43, Ali Afshar wrote:
> > Add a docstring, remove the pass (or put on a new line). And need another
> > linefeed above.
> 
> Done.
> 
> http://codereview.appspot.com/4956059/diff/7001/oauth2client/client.py
> File oauth2client/client.py (right):
> 
>
http://codereview.appspot.com/4956059/diff/7001/oauth2client/client.py#newcod...
> oauth2client/client.py:768: sys.exit(message)
> On 2011/10/06 23:53:43, Ali Afshar wrote:
> > Is this sys.exit really necessary? It could cause a world of pain by
accident.
> 
> Done.
> 
> http://codereview.appspot.com/4956059/diff/7001/oauth2client/clientsecrets.py
> File oauth2client/clientsecrets.py (right):
> 
>
http://codereview.appspot.com/4956059/diff/7001/oauth2client/clientsecrets.py...
> oauth2client/clientsecrets.py:76: if len(obj) != 1:
> On 2011/10/06 23:53:43, Ali Afshar wrote:
> > This could fail if obj is None, which it could. Perhaps add another check
for
> > None?
> 
> Done.
> 
>
http://codereview.appspot.com/4956059/diff/7001/tests/test_oauth2client_clien...
> File tests/test_oauth2client_clientsecrets.py (right):
> 
>
http://codereview.appspot.com/4956059/diff/7001/tests/test_oauth2client_clien...
> tests/test_oauth2client_clientsecrets.py:37: ERRORS = [
> On 2011/10/06 23:53:43, Ali Afshar wrote:
> > Add test for 'null' which returns None and would break the validator
> currently.
> 
> Done.
> 
>
http://codereview.appspot.com/4956059/diff/7001/tests/test_oauth2client_clien...
> tests/test_oauth2client_clientsecrets.py:66: except
> clientsecrets.InvalidClientSecretsError, e:
> On 2011/10/06 23:53:43, Ali Afshar wrote:
> > any reason for not using assertRaises?
> 
> Yes, because I also want to check message that comes along with the exception.
> 
>
http://codereview.appspot.com/4956059/diff/7001/tests/test_oauth2client_clien...
> tests/test_oauth2client_clientsecrets.py:74: except
> clientsecrets.InvalidClientSecretsError, e:
> On 2011/10/06 23:53:43, Ali Afshar wrote:
> > ditto re: assertRaises
> 
> Done.

Expand All Messages | Collapse All Messages