Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(29)

Issue 6265043: Make oauth2client support Windows-friendly locking

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 1 month ago by cache
Modified:
13 years ago
Reviewers:
jcgregorio_google
CC:
google-api-python-client_googlegroups.com
Visibility:
Public.

Patch Set 1 #

Total comments: 8

Patch Set 2 : Addressing comments for locked_file.py #

Total comments: 9

Patch Set 3 : Further updates to locked_file.py #

Total comments: 2

Patch Set 4 : Fix broken comment. #

Total comments: 4

Patch Set 5 : Fix _Opener misname. #

Patch Set 6 : Fix EACCES handling in FcntlOpener. #

Total comments: 2

Patch Set 7 : Remove unintended change to test_oauth2client_file.py #

Total comments: 8

Patch Set 8 : Update docs. #

Total comments: 4

Patch Set 9 : Safer close() handling. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+268 lines, -23 lines) Patch
A oauth2client/locked_file.py View 1 2 3 4 5 6 7 8 1 chunk +254 lines, -0 lines 0 comments Download
M oauth2client/multistore_file.py View 7 chunks +14 lines, -23 lines 0 comments Download

Messages

Total messages: 17
jcgregorio_google
http://codereview.appspot.com/6265043/diff/1/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/1/oauth2client/locked_file.py#newcode6 oauth2client/locked_file.py:6: to a file, then falls back on a lock ...
13 years, 1 month ago (2012-06-05 21:10:02 UTC) #1
cache
http://codereview.appspot.com/6265043/diff/1/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/1/oauth2client/locked_file.py#newcode6 oauth2client/locked_file.py:6: to a file, then falls back on a lock ...
13 years, 1 month ago (2012-06-07 19:26:22 UTC) #2
jcgregorio_google
http://codereview.appspot.com/6265043/diff/2002/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/2002/oauth2client/locked_file.py#newcode41 oauth2client/locked_file.py:41: filename: The pathname of the file. Add types to ...
13 years, 1 month ago (2012-06-07 19:53:54 UTC) #3
cache
PTAL http://codereview.appspot.com/6265043/diff/2002/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/2002/oauth2client/locked_file.py#newcode41 oauth2client/locked_file.py:41: filename: The pathname of the file. On 2012/06/07 ...
13 years ago (2012-06-08 18:53:09 UTC) #4
jcgregorio_google
lgtm
13 years ago (2012-06-11 13:32:03 UTC) #5
jcgregorio_google
http://codereview.appspot.com/6265043/diff/8002/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/8002/oauth2client/locked_file.py#newcode135 oauth2client/locked_file.py:135: """ Spoke too soon with the lgtm. This isn't ...
13 years ago (2012-06-11 13:34:47 UTC) #6
cache
http://codereview.appspot.com/6265043/diff/8002/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/8002/oauth2client/locked_file.py#newcode135 oauth2client/locked_file.py:135: """ Gah. What happened there? Done.
13 years ago (2012-06-11 19:52:03 UTC) #7
jcgregorio_google
http://codereview.appspot.com/6265043/diff/4003/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/4003/oauth2client/locked_file.py#newcode76 oauth2client/locked_file.py:76: class _PosixOpener(Opener): _Opener http://codereview.appspot.com/6265043/diff/4003/oauth2client/locked_file.py#newcode131 oauth2client/locked_file.py:131: class _FcntlOpener(Opener): _Opener
13 years ago (2012-06-11 20:39:17 UTC) #8
cache
http://codereview.appspot.com/6265043/diff/4003/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/4003/oauth2client/locked_file.py#newcode76 oauth2client/locked_file.py:76: class _PosixOpener(Opener): Aha. My test didn't actually work since ...
13 years ago (2012-06-11 20:59:32 UTC) #9
jcgregorio_google
I patched this in and the tests in test_oauth2client_file.py are freezing under linux. On 2012/06/11 ...
13 years ago (2012-06-11 21:11:27 UTC) #10
cache
Sadly it wasn't obvious how to make the tests run (I didn't have appengine-sdk), but ...
13 years ago (2012-06-11 22:29:45 UTC) #11
jcgregorio_google
http://codereview.appspot.com/6265043/diff/2006/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/2006/oauth2client/locked_file.py#newcode146 oauth2client/locked_file.py:146: try: This same strategy, of trying to open self._fh ...
13 years ago (2012-06-12 12:33:34 UTC) #12
cache
http://codereview.appspot.com/6265043/diff/2006/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/2006/oauth2client/locked_file.py#newcode146 oauth2client/locked_file.py:146: try: On 2012/06/12 12:33:34, jcgregorio_google wrote: > This same ...
13 years ago (2012-06-12 18:23:26 UTC) #13
jcgregorio_google
http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py#newcode56 oauth2client/locked_file.py:56: return self._fh Define self._fh in __init__. Ran into this ...
13 years ago (2012-06-12 20:54:40 UTC) #14
cache
http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py File oauth2client/locked_file.py (right): http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py#newcode56 oauth2client/locked_file.py:56: return self._fh On 2012/06/12 20:54:40, jcgregorio_google wrote: > Define ...
13 years ago (2012-06-12 21:10:20 UTC) #15
jcgregorio_google
lgtm On 2012/06/12 21:10:20, cache wrote: > http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py > File oauth2client/locked_file.py (right): > > http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py#newcode56 ...
13 years ago (2012-06-13 16:14:32 UTC) #16
jcgregorio_google
13 years ago (2012-06-13 16:19:27 UTC) #17
Committed in
http://code.google.com/p/google-api-python-client/source/detail?r=ac147884bf0...

On 2012/06/13 16:14:32, jcgregorio_google wrote:
> lgtm
> 
> On 2012/06/12 21:10:20, cache wrote:
> > http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py
> > File oauth2client/locked_file.py (right):
> > 
> >
>
http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py#...
> > oauth2client/locked_file.py:56: return self._fh
> > On 2012/06/12 20:54:40, jcgregorio_google wrote:
> > > Define self._fh in __init__.
> > > 
> > > Ran into this in local testing.
> > 
> > Done.
> > 
> >
>
http://codereview.appspot.com/6265043/diff/11003/oauth2client/locked_file.py#...
> > oauth2client/locked_file.py:121: self._fh.close()
> > On 2012/06/12 20:54:40, jcgregorio_google wrote:
> > > It may be possible that self._fh is None here, check for that before
calling
> > > close().
> > > 
> > > Here and below.
> > 
> > Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b