Retrying when we encounter a connection reset

Review Request #3563 — Created Nov. 26, 2012 and discarded

Information

RBTools

Reviewers

Our users have occasionally encountered connection resets when running postreview.py from remote locations with poor connectivity. These errors are transient, so adding retries when postreview encounters a timeout.

This is available in my timeout_retries branch...

https://github.com/atagar/rbtools/commit/08c288484576503768c29e87c9b6cb08529fec90
https://github.com/atagar/rbtools/tree/timeout_retries
This exact change is untested, but we've been running an identical change with RBTools 0.3 for three months now without any issues.
Description From Last Updated

Col: 80 E501 line too long (87 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (108 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (100 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot

Col: 16 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 16 E113 unexpected indentation

reviewbotreviewbot

Col: 18 E111 indentation is not a multiple of four

reviewbotreviewbot

Col: 18 E113 unexpected indentation

reviewbotreviewbot

Col: 17 E901 IndentationError

reviewbotreviewbot

Col: 17 E113 unexpected indentation

reviewbotreviewbot

Col: 17 E112 expected an indented block

reviewbotreviewbot

Please add a docstring for this method.

daviddavid

Instead of using the continuation character \, please wrap the condition in parentheses.

daviddavid

Why the sleep? This should also probably have a handler for the case where tries == 0, and show an …

daviddavid
AT
  1. This and my other RBTools patches (http://reviews.reviewboard.org/r/3221/ and http://reviews.reviewboard.org/r/3208/) have collected dust for three to seven months. Is RBTools no longer maintained?
    1. RBTools is being maintained, but it's been undergoing a significant rewrite. There will be a 0.5 release out soon that drastically changes the codebase. One of the main things is that communication code is now common and not isolated to postreview.py (though postreview.py is staying around mostly as-is for backwards-compatibility reasons for another release or two).
      
      RBTools has historically been less actively developed than Review Board (it's primarily been 2 people total for the majority of all our development), but we have a dedicated maintainer now for RBTools.
    2. > There will be a 0.5 release out soon that drastically changes the codebase.
      
      Neat, glad to hear it! Any guess roughly when it'll be released? Re-applying our patch set to 0.4 has been slowly crawling up my todo list but if 0.5 is just around the corner then I might as well wait for that (and update these code reviews with the rebased versions).
    3. We're working to get the documentation in shape now, and fix any bugs that crop up, so it's pretty close.
  2. 
      
AT
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/postreview.py
      Ignored Files:
    
    
  2. rbtools/postreview.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (87 > 79 characters)
    
  3. rbtools/postreview.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (108 > 79 characters)
    
  4. rbtools/postreview.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (100 > 79 characters)
    
  5. rbtools/postreview.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (95 > 79 characters)
    
  6. 
      
AT
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/postreview.py
      Ignored Files:
    
    
  2. 
      
AT
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/postreview.py
      Ignored Files:
    
    
  2. rbtools/postreview.py (Diff revision 4)
     
     
    Show all issues
    Col: 16
     E111 indentation is not a multiple of four
    
  3. rbtools/postreview.py (Diff revision 4)
     
     
    Show all issues
    Col: 16
     E113 unexpected indentation
    
  4. 
      
AT
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/postreview.py
      Ignored Files:
    
    
  2. rbtools/postreview.py (Diff revision 5)
     
     
    Show all issues
    Col: 18
     E111 indentation is not a multiple of four
    
  3. rbtools/postreview.py (Diff revision 5)
     
     
    Show all issues
    Col: 18
     E113 unexpected indentation
    
  4. rbtools/postreview.py (Diff revision 5)
     
     
    Show all issues
    Col: 17
     E901 IndentationError
  5. 
      
AT
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/postreview.py
      Ignored Files:
    
    
  2. rbtools/postreview.py (Diff revision 6)
     
     
    Show all issues
    Col: 17
     E113 unexpected indentation
    
  3. rbtools/postreview.py (Diff revision 6)
     
     
    Show all issues
    Col: 17
     E112 expected an indented block
    
  4. 
      
AT
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        rbtools/postreview.py
      Ignored Files:
    
    
  2. 
      
david
  1. Sorry for the long delay on this.
  2. rbtools/postreview.py (Diff revision 7)
     
     
    Show all issues
    Please add a docstring for this method.
  3. rbtools/postreview.py (Diff revision 7)
     
     
     
    Show all issues
    Instead of using the continuation character \, please wrap the condition in parentheses.
  4. rbtools/postreview.py (Diff revision 7)
     
     
    Show all issues
    Why the sleep?
    
    This should also probably have a handler for the case where tries == 0, and show an error that it tried and gave up.
  5. 
      
AT
Review request changed
Status:
Discarded