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

Issue 6912052: Catch timeout errors in upload.py and retry

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 10 months ago by hinoka
Modified:
12 years, 10 months ago
Reviewers:
M-A Ruel, M-A
CC:
codereview-discuss_googlegroups.com
Visibility:
Public.

Description

Started as: https://codereview.chromium.org/11446068/ Catching urllib2.URLError These sometimes occur when an intermediate proxy is overloaded, and is only an temporary issue. We want to catch these exceptions and retry uploading.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -0 lines) Patch
M third_party/upload.py View 2 chunks +13 lines, -0 lines 2 comments Download

Messages

Total messages: 2
hinoka
12 years, 10 months ago (2012-12-07 21:35:41 UTC) #1
M-A
12 years, 10 months ago (2012-12-10 15:08:44 UTC) #2
https://codereview.appspot.com/6912052/diff/1/third_party/upload.py File third_party/upload.py (right): https://codereview.appspot.com/6912052/diff/1/third_party/upload.py#newcode424 third_party/upload.py:424: elif e.reason == '[Errno 110] Connection timed out': I'm not sure the error code is portable. So I'd prefer with elif 'Connection timed out' in e.reason: but see my comment below. https://codereview.appspot.com/6912052/diff/1/third_party/upload.py#newcode431 third_party/upload.py:431: raise e "raise" is better than "raise e" since it re-raise the original exception. That's what you did at line 423. I don't think it's useful to have 3 condition blocks, you really only need one; if 'Connection timed out' in e.reason and tries <= 3: # retry with backoff (...) continue raise that's it.
Sign in to reply to this message.

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