Make maxtries for Rietveld configurable

rietveld.py

 # coding: utf-8
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Defines class Rietveld to easily access a rietveld instance.
 
 Security implications:
 
 The following hypothesis are made:
 - Rietveld enforces:
@@ skipping 19 matching lines @@
 import third_party.oauth2client.client as oa2client
 from third_party import httplib2
 
 # Appengine replies with 302 when authentication fails (sigh.)
 oa2client.REFRESH_STATUS_CODES.append(302)
 upload.LOGGER.setLevel(logging.WARNING)  # pylint: disable=E1103
 
 
 class Rietveld(object):
   """Accesses rietveld."""
-  def __init__(self, url, email, password, extra_headers=None):
+  def __init__(self, url, email, password, extra_headers=None, maxtries=None):
     self.url = url.rstrip('/')
     # Email and password are accessed by commit queue, keep them.
     self.email = email
     self.password = password
     # TODO(maruel): It's not awesome but maybe necessary to retrieve the value.
     # It happens when the presubmit check is ran out of process, the cookie
     # needed to be recreated from the credentials. Instead, it should pass the
     # email and the cookie.
     if email and password:
       get_creds = lambda: (email, password)
       self.rpc_server = upload.HttpRpcServer(
             self.url,
             get_creds,
             extra_headers=extra_headers or {})
     else:
       if email == '':
         # If email is given as an empty string, then assume we want to make
         # requests that do not need authentication.  Bypass authentication by
         # setting the auth_function to None.
         self.rpc_server = upload.HttpRpcServer(url, None)
       else:
         self.rpc_server = upload.GetRpcServer(url, email)
 
     self._xsrf_token = None
     self._xsrf_token_time = None
 
+    self._maxtries = maxtries or 40
+
   def xsrf_token(self):
     if (not self._xsrf_token_time or
         (time.time() - self._xsrf_token_time) > 30*60):
       self._xsrf_token_time = time.time()
       self._xsrf_token = self.get(
           '/xsrf_token',
           extra_headers={'X-Requesting-XSRF-Token': '1'})
     return self._xsrf_token
 
   def get_pending_issues(self):
@@ skipping 329 matching lines @@
       def trap_http_500(msg):
         """Converts an incorrect ErrorExit() call into a HTTPError exception."""
         m = re.search(r'(50\d) Server Error', msg)
         if m:
           # Fake an HTTPError exception. Cheezy. :(
           raise urllib2.HTTPError(
               request_path, int(m.group(1)), msg, None, None)
         old_error_exit(msg)
       upload.ErrorExit = trap_http_500
 
-      maxtries = 40
-      for retry in xrange(maxtries):
+      for retry in xrange(self._maxtries):
         try:
           logging.debug('%s' % request_path)
           result = self.rpc_server.Send(request_path, **kwargs)
           # Sometimes GAE returns a HTTP 200 but with HTTP 500 as the content.
           # How nice.
           return result
         except urllib2.HTTPError, e:
-          if retry >= (maxtries - 1):
+          if retry >= (self._maxtries - 1):
             raise
           flake_codes = [500, 502, 503]
           if retry_on_404:
             flake_codes.append(404)
           if e.code not in flake_codes:
             raise
         except urllib2.URLError, e:
-          if retry >= (maxtries - 1):
+          if retry >= (self._maxtries - 1):
             raise
           if (not 'Name or service not known' in e.reason and
               not 'EOF occurred in violation of protocol' in e.reason):
             # Usually internal GAE flakiness.
             raise
         except ssl.SSLError, e:
-          if retry >= (maxtries - 1):
+          if retry >= (self._maxtries - 1):
             raise
           if not 'timed out' in str(e):
             raise
         # If reaching this line, loop again. Uses a small backoff.
         time.sleep(min(10, 1+retry*2))
     finally:
       upload.ErrorExit = old_error_exit
 
   # DEPRECATED.
   Send = get
@@ skipping 117 matching lines @@
   This class is supposed to be used only by bots, since this kind of
   access is restricted to service accounts.
   """
   # The parent__init__ is not called on purpose.
   # pylint: disable=W0231
   def __init__(self,
                url,
                client_email,
                client_private_key_file,
                private_key_password=None,
-               extra_headers=None):
+               extra_headers=None,
+               maxtries=None):
 
     # These attributes are accessed by commit queue. Keep them.
     self.email = client_email
     self.private_key_file = client_private_key_file
 
     if private_key_password is None:  # '' means 'empty password'
       private_key_password = 'notasecret'
 
     self.url = url.rstrip('/')
     bot_url = self.url + '/bots'
 
     with open(client_private_key_file, 'rb') as f:
       client_private_key = f.read()
     logging.info('Using OAuth login: %s' % client_email)
     self.rpc_server = OAuthRpcServer(bot_url,
                                      client_email,
                                      client_private_key,
                                      private_key_password=private_key_password,
                                      extra_headers=extra_headers or {})
     self._xsrf_token = None
     self._xsrf_token_time = None
 
+    self._maxtries = 40 or maxtries

[Michael Achenbach, 2015/02/19 11:20:33]

Hmm, sad that there's no super call.

[Jens Widell, 2015/02/19 11:32:39]

This is the same as "self._maxtries = 40". Did you mean "self._maxtries =
maxtries or 40"?

[Michael Achenbach, 2015/02/19 11:33:56]

Ups, totally missed that. Good catch!

[Paweł Hajdan Jr., 2015/02/19 11:49:04]

Good catch, fixed in https://codereview.chromium.org/933373004/

Thanks!

+
 
 class CachingRietveld(Rietveld):
   """Caches the common queries.
 
   Not to be used in long-standing processes, like the commit queue.
   """
   def __init__(self, *args, **kwargs):
     super(CachingRietveld, self).__init__(*args, **kwargs)
     self._cache = {}
 
@@ skipping 114 matching lines @@
       self, issue, patchset, reason, clobber, revision, builders_and_tests,
       master=None, category='cq'):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (builders_and_tests, issue))
 
   def trigger_distributed_try_jobs(  # pylint:disable=R0201
       self, issue, patchset, reason, clobber, revision, masters,
       category='cq'):
     logging.info('ReadOnlyRietveld: triggering try jobs %r for issue %d' %
         (masters, issue))