Issue 9363044: Add threadsafety to oauth2decorator.

Issue 9363044: Add threadsafety to oauth2decorator. (Closed)

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

Created:
12 years, 1 month ago by jcgregorio_google

Modified:
12 years, 1 month ago

Reviewers:
jbeda_google, jcgregorio_google, dhermes, joebeda

CC:
google-api-python-client_googlegroups.com

Visibility:
Public.

More Reviews

Patch Set 3 : fix depth of diff #

Patch Set 5 : Clear credentials after each call. #

Total comments: 4

Patch Set 6 : Handle exceptions. #

Patch Set 7 : typo #

Total comments: 1

Created: 12 years, 1 month ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+85 lines, -14 lines)			Patch
	M	oauth2client/appengine.py	View	1 2 3 4 5	5 chunks	+28 lines, -2 lines	1 comment	Download
	M	tests/test_oauth2client_appengine.py	View	1 2 3 4 5 6	6 chunks	+57 lines, -12 lines	0 comments	Download

Messages

Total messages: 6

Expand All Messages | Collapse All Messages

jbeda_google

https://codereview.appspot.com/9363044/diff/9001/oauth2client/appengine.py File oauth2client/appengine.py (right): https://codereview.appspot.com/9363044/diff/9001/oauth2client/appengine.py#newcode700 oauth2client/appengine.py:700: self.credentials = None Move this to a finally block? ...

12 years, 1 month ago (2013-05-15 14:16:34 UTC) #2

jcgregorio_google

12 years, 1 month ago (2013-05-15 14:40:32 UTC) #3

joebeda

LGTM -- Thanks Joe! https://codereview.appspot.com/9363044/diff/15001/oauth2client/appengine.py File oauth2client/appengine.py (right): https://codereview.appspot.com/9363044/diff/15001/oauth2client/appengine.py#newcode699 oauth2client/appengine.py:699: resp = method(request_handler, *args, **kwargs) ...

12 years, 1 month ago (2013-05-16 18:39:45 UTC) #4

jcgregorio_google

Committed in https://code.google.com/p/google-api-python-client/source/detail?r=681174478ec1d0f0228a8930bc37d0796dd76500 On 2013/05/16 18:39:45, joebeda wrote: > LGTM -- Thanks Joe! > > ...

12 years, 1 month ago (2013-05-16 19:57:46 UTC) #5

jbeda_google

12 years, 1 month ago (2013-05-22 05:15:45 UTC) #6

One more comment on this patch.  The getters should handle the case where
there is no value set on the TLS object.  Basically:
  def set_credentials(self, credentials):
    self._tls.credentials = credentials

  def get_credentials(self):
    if hasattr(self._tls, 'credentials'):
      return self._tls.credentials
    return None

  def set_flow(self, flow):
    self._tls.flow = flow

  def get_flow(self):
    if hasattr(self._tls, 'flow'):
      return self._tls.flow
    return None

I'd send you an hg patch but I don't have time to relearn hg tonight :)


On Thu, May 16, 2013 at 12:57 PM, <jcgregorio@google.com> wrote:

> Committed in
> https://code.google.com/p/**google-api-python-client/**source/detail?r=**
>
681174478ec1d0f0228a8930bc37d0**796dd76500<https://code.google.com/p/google-api-python-client/source/detail?r=681174478ec1d0f0228a8930bc37d0796dd76500>
>
>
> On 2013/05/16 18:39:45, joebeda wrote:
>
>> LGTM -- Thanks Joe!
>>
>
>
> https://codereview.appspot.**com/9363044/diff/15001/**
>
oauth2client/appengine.py<https://codereview.appspot.com/9363044/diff/15001/oauth2client/appengine.py>
>
>> File oauth2client/appengine.py (right):
>>
>
>
> https://codereview.appspot.**com/9363044/diff/15001/**
>
oauth2client/appengine.py#**newcode699<https://codereview.appspot.com/9363044/diff/15001/oauth2client/appengine.py#newcode699>
>
>> oauth2client/appengine.py:699: resp = method(request_handler, *args,
>>
> **kwargs)
>
>> Nit, but no need to store into resp.  You can just return it here like
>>
> the
>
>> original version.
>>
>
>
>
https://codereview.appspot.**com/9363044/<https://codereview.appspot.com/9363...
>

Expand All Messages | Collapse All Messages