Merge lp:~javier.collado/utah/bug1103448 into lp:utah

[Javier Collado]

This branch implements a workaround for http errors like the ones described in
bug1103448.

The workaround is just retry the failed `bzr export` commands for one minute
every 3 seconds.

The values have been hardcoded, but they could be added to the configuration
file if needed. Anyway, given that the change affects both the client and the
server, I think this could be updated only on the server since the client would
always get the defaults from the configuration file in the package when
installed. Please let me know if there's some good way to share this
configuration data without adding a new command line option at the client.

Aside from this, I've spent some time improving the documentation for timeout
and retry since that has been helpful when implementing the workaround.

[Max Brustkern (nuclearbob)]

Looks good to me. I haven't tested it yet, but I can do that after I finish the other testing I'm doing.

review: Approve

=== modified file 'utah/client/runner.py'
--- utah/client/runner.py	2012-12-12 11:21:10 +0000
+++ utah/client/runner.py	2013-02-06 11:14:36 +0000
@@ -18,12 +18,16 @@
 from utah.client.testsuite import TestSuite
 from utah.client.state_agent import StateAgentYAML
 from utah.client import exceptions
+from utah.exceptions import UTAHException
+from utah.timeout import timeout
+from utah.retry import retry
 
 import os
 import shutil
 import stat
 import urllib
 import jsonschema
+import time
 
 from utah.client.common import (
     MASTER_RUNLIST,
@@ -443,9 +447,20 @@
             if not os.path.exists(name):
                 os.mkdir(name)
 
+            def vcs_get_retriable():
+                result = vcs_handler.get(directory=name)
+                if (isinstance(vcs_handler, BzrHandler) and
+                        result['returncode'] == 3 and
+                        'Unable to handle http code 502' in result['stderr']):
+                    # Separate every retry attempt with a timeout of 3 seconds
+                    time.sleep(3)
+                    raise UTAHException('Launchpad temporary error detected',
+                                        retry=True)
+                return result
+
             if name not in self.fetched_suites:
-
-                res = vcs_handler.get(directory=name)
+                # Retry bzr export on http errors for 60 seconds
+                res = timeout(60, retry, vcs_get_retriable)
                 self.result.add_result(res)
 
                 # If fetch failed move on to the next testsuite.

=== modified file 'utah/retry.py'
--- utah/retry.py	2012-12-03 14:02:18 +0000
+++ utah/retry.py	2013-02-06 11:14:36 +0000
@@ -23,9 +23,26 @@
 
 
 def retry(command, *args, **kw):
-    """
-    Retry an operation until it completes or an exception is raised without
-    retry set to True.
+    """Retry a command as long as a retriable exception is captured.
+
+    A retriable exception is considered to be a
+    ``UTAHException`` that has the attribute ``retry`` set to
+    ``True``.
+
+    :param command: Command to be executed.
+    :type command: callable
+    :param args: Positional arguments to be passed to the callable.
+    :param kwargs:
+        Keyword arguments to be passed to the callable.
+
+        .. note::
+            If a keyword argument named ``logmethod`` is found, it will be
+            extracted (i.e. it won't be passed to the callable) and used to log
+            every retry attempt (using ``sys.stderr`` by default).
+    :returns: The value returned by the callable.
+
+    .. seealso:: :func:`utah.timeout.timeout`
+
     """
     try:
         logmethod = kw.pop('logmethod')

=== modified file 'utah/timeout.py'
--- utah/timeout.py	2012-12-08 02:10:12 +0000
+++ utah/timeout.py	2013-02-06 11:14:36 +0000
@@ -25,18 +25,29 @@
 
 
 class UTAHTimeout(UTAHException):
-    """
-    Provide a special exception to indicate an operation timed out.
-    """
+    """Provide a special exception to indicate an operation timed out."""
 
 
 # Mostly pulled from utah/client/common.py
 # Inspired by:
 # http://stackoverflow.com/a/3326559
 def timeout(timeout, command, *args, **kw):
-    """
-    Run command with all other arguments for up to timeout seconds, after with
-    a UTAHTimeout exception is returned.
+    """Run a command for up to ``timeout`` seconds.
+
+    :param timeout:
+        Maximum amount of time to wait for the command to complete in seconds.
+    :type timeout: int
+    :param command:
+        Command whose execution should complete before timeout expires.
+    :type command: callable
+    :param args: Positional arguments to be passed to the callable.
+    :param kwargs: Keyword arguments to be passed to the callable.
+    :returns: The value returned by the callable.
+    :raises UTAHTimeout:
+        If command execution hasn't finished before ``timeout`` seconds.
+
+    .. seealso:: :func:`utah.retry.retry`
+
     """
     #TODO: Better support for nested timeouts.
     if command is None:
@@ -71,11 +82,20 @@
 
 
 def subprocesstimeout(timeout, *args, **kw):
-    """
-    Pass all arguments except timeout to subprocess.Popen and run for timeout
-    seconds.
-    After the timeout is reached, kill all subprocesses and return a
-    UTAHTimeout exception.
+    """Run command through ``subprocess.Popen`` for up to ``timeout`` seconds.
+
+    Command termination is checked using ``subprocess.Popen.poll``.
+
+    :param timeout:
+        Maximum amount of time to wait for the command to complete in seconds.
+    :type timeout: int
+    :param args: Positional arguments to be passed to ``subprocess.Popen``.
+    :param kwargs: Keyword arguments to be passed to the ``subprocess.Popen``.
+    :returns: The subprocess object
+    :rtype: ``subprocess.Popen``
+    :raises UTAHTimeout:
+        If command execution hasn't finished before ``timeout`` seconds.
+
     """
     if args is None:
         return
@@ -118,8 +138,13 @@
 
 
 def get_process_children(pid):
-    """
-    Find process children so they can be killed when the timeout expires.
+    """Find process children so they can be killed when the timeout expires.
+
+    :param pid: Process ID for the parent process
+    :type pid: int
+    :returns: The pid for each children process
+    :rtype: list(int)
+
     """
     p = subprocess.Popen('ps --no-headers -o pid --ppid %d' % pid, shell=True,
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)

=== modified file 'utah/url.py'
--- utah/url.py	2012-12-13 13:48:54 +0000
+++ utah/url.py	2013-02-06 11:14:36 +0000
@@ -24,6 +24,7 @@
 import urllib2
 import tempfile
 import logging
+import time
 from urlparse import urlparse
 from argparse import ArgumentTypeError
 
@@ -32,6 +33,8 @@
 import bzrlib.errors
 
 from utah.exceptions import UTAHException
+from utah.timeout import timeout
+from utah.retry import retry
 
 
 # Inspired by: http://stackoverflow.com/a/2070916/183066
@@ -212,8 +215,25 @@
         cmd = bzrlib.builtins.cmd_export()
         assert cmd is not None
         tmp_dir = tempfile.mkdtemp(prefix='utah_')
+
+        def bzr_export_retriable():
+            """bzr export a URL retrying on http errors
+
+            This is a workaround to launchpad problems with http URLs that
+            happen from time to time
+
+            """
+            try:
+                cmd.run(tmp_dir, url)
+            except bzrlib.errors.InvalidHttpResponse as exception:
+                # Separate every retry attempt with a timeout of 3 seconds
+                time.sleep(3)
+                raise UTAHException(exception.path, exception.msg, retry=True)
+
         try:
-            cmd.run(tmp_dir, url)
+            # Retry bzr export on http errors for 60 seconds
+            timeout(60, retry, bzr_export_retriable,
+                    logmethod=bzr_logger.debug)
         except bzrlib.errors.BzrError as exception:
             raise ArgumentTypeError('Bazaar export error: {}'
                                     .format(exception))