Various tweaks to pip downloading

- Move the pip dependency downloading function + logic
  to the pip_helper module instead of being in the base.py
  module (since its a pip utility function)
- Add a retry function to utils and use it instead of the
  custom retry module used for downloading pip dependencies.
- Cleanup other various code in pip_helper.py

Change-Id: I038e4d73c814ea06a18eae61e41c5b7bef779b9d

anvil/packaging/base.py

@@ -52,6 +52,7 @@ class InstallHelper(object):
 class DependencyHandler(object):
     """Basic class for handler of OpenStack dependencies."""
     MAX_PIP_DOWNLOAD_ATTEMPTS = 4
+    PIP_DOWNLOAD_DELAY = 10
 
     def __init__(self, distro, root_dir, instances, opts):
         self.distro = distro
@@ -69,7 +70,6 @@ class DependencyHandler(object):
         self.download_requires_filename = sh.joinpths(self.deps_dir, "download-requires")
         # Executables we require to operate
         self.multipip_executable = sh.which("multipip", ["tools/"])
-        self.pip_executable = sh.which_first(['pip', 'pip-python'])
         # List of requirements
         self.pips_to_install = []
         self.forced_packages = []
@@ -249,31 +249,6 @@ class DependencyHandler(object):
         """
         return self.pips_to_install
 
-    def _try_download_dependencies(self, attempt, pips_to_download, pip_download_dir):
-        # Clean out any previous paths that we don't want around.
-        for path in ['.build']:
-            path = sh.joinpths(pip_download_dir, path)
-            if sh.isdir(path):
-                sh.deldir(path)
-            sh.mkdir(path)
-        # Ensure certain directories exist that we want to exist (but we don't
-        # want to delete them run after run).
-        for path in ['.cache']:
-            path = sh.joinpths(pip_download_dir, path)
-            if not sh.isdir(path):
-                sh.mkdir(path)
-        cmdline = [
-            self.pip_executable, '-v',
-            'install', '-I', '-U',
-            '--download', pip_download_dir,
-            '--build', sh.joinpths(pip_download_dir, '.build'),
-            '--download-cache', sh.joinpths(pip_download_dir, '.cache'),
-        ]
-        cmdline.extend(sorted([str(p) for p in pips_to_download]))
-        out_filename = sh.joinpths(self.log_dir,
-                                   "pip-download-attempt-%s.log" % (attempt))
-        sh.execute_save_output(cmdline, out_filename)
-
     def _examine_download_dir(self, pips_to_download, pip_download_dir):
         pip_names = set([p.key for p in pips_to_download])
         what_downloaded = sh.listdir(pip_download_dir, files_only=True)
@@ -284,6 +259,7 @@ class DependencyHandler(object):
             if req.key not in pip_names:
                 LOG.info("Dependency %s was automatically included.",
                          colorizer.quote(req))
+        return what_downloaded
 
     @staticmethod
     def _requirements_satisfied(pips_list, download_dir):
@@ -316,30 +292,17 @@ class DependencyHandler(object):
                 self._requirements_satisfied(pips_to_download, self.download_dir)):
             LOG.info("All python dependencies have been already downloaded")
         else:
-            pip_failures = []
+            def try_download(attempt):
-            for attempt in xrange(self.MAX_PIP_DOWNLOAD_ATTEMPTS):
+                output_filename = sh.joinpths(self.log_dir,
-                # NOTE(aababilov): pip has issues with already downloaded files
+                                              "pip-download-attempt-%s.log" % (attempt))
-                for filename in sh.listdir(self.download_dir, files_only=True):
+                pip_helper.download_dependencies(self.download_dir,
-                    sh.unlink(filename)
+                                                 pips_to_download,
-                header = "Downloading %s python dependencies (attempt %s)"
+                                                 output_filename)
-                header = header % (len(pips_to_download), attempt + 1)
+            utils.retry(self.MAX_PIP_DOWNLOAD_ATTEMPTS,
-                utils.log_iterable(sorted(pips_to_download), logger=LOG, header=header)
+                        self.PIP_DOWNLOAD_DELAY, try_download)
-                failed = False
-                try:
-                    self._try_download_dependencies(attempt + 1, pips_to_download,
-                                                    self.download_dir)
-                    pip_failures = []
-                except exc.ProcessExecutionError as e:
-                    LOG.exception("Failed downloading python dependencies")
-                    pip_failures.append(e)
-                    failed = True
-                if not failed:
-                    break
-            if pip_failures:
-                raise pip_failures[-1]
             # NOTE(harlowja): Mark that we completed downloading successfully
             sh.touch_file(self.downloaded_flag_file, die_if_there=False,
                           quiet=True, tracewriter=self.tracewriter)
         pips_downloaded = [pip_helper.extract_requirement(p) for p in pips_to_download]
-        self._examine_download_dir(pips_downloaded, self.download_dir)
+        what_downloaded = self._examine_download_dir(pips_downloaded, self.download_dir)
-        return (pips_downloaded, sh.listdir(self.download_dir, files_only=True))
+        return (pips_downloaded, what_downloaded)

anvil/packaging/helpers/pip_helper.py

@@ -14,6 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from distutils import version as dist_version
 import pkg_resources
 import re
 
@@ -26,9 +27,12 @@ from anvil import utils
 
 LOG = logging.getLogger(__name__)
 
-FREEZE_CMD = ['freeze', '--local']
 EGGS_DETAILED = {}
 PYTHON_KEY_VERSION_RE = re.compile("^(.+)-([0-9][0-9.a-zA-Z]*)$")
+PIP_VERSION = pkg_resources.get_distribution('pip').version
+PIP_EXECUTABLE = sh.which_first(['pip', 'pip-python'])
+OPENSTACK_TARBALLS_RE = re.compile(r'http://tarballs.openstack.org/([^/]+)/')
+SKIP_LINES = ('#', '-e', '-f', 'http://', 'https://')
 
 
 def create_requirement(name, version=None):
@@ -124,22 +128,16 @@ def get_archive_details(filename):
     return details
 
 
-SKIP_LINES = ('#', '-e', '-f', 'http://', 'https://')
-
-
 def _skip_requirement(line):
     return not len(line) or any(line.startswith(a) for a in SKIP_LINES)
 
 
-OPESTACK_TARBALLS_RE = re.compile(r'http://tarballs.openstack.org/([^/]+)/')
-
-
 def parse_requirements(contents, adjust=False):
     lines = []
     for line in contents.splitlines():
         line = line.strip()
         if 'http://' in line:
-            m = OPESTACK_TARBALLS_RE.search(line)
+            m = OPENSTACK_TARBALLS_RE.search(line)
             if m:
                 line = m.group(1)
         if not _skip_requirement(line):
@@ -155,3 +153,38 @@ def read_requirement_files(files):
             with open(filename) as f:
                 result.extend(parse_requirements(f.read()))
     return result
+
+
+def download_dependencies(download_dir, pips_to_download, output_filename):
+    if not pips_to_download:
+        return
+    # NOTE(aababilov): pip has issues with already downloaded files
+    if sh.isdir(download_dir):
+        for filename in sh.listdir(download_dir, files_only=True):
+            sh.unlink(filename)
+    else:
+        sh.mkdir(download_dir)
+    # Clean out any previous paths that we don't want around.
+    build_path = sh.joinpths(download_dir, ".build")
+    if sh.isdir(build_path):
+        sh.deldir(build_path)
+    sh.mkdir(build_path)
+    # Ensure certain directories exist that we want to exist (but we don't
+    # want to delete them run after run).
+    cache_path = sh.joinpths(download_dir, ".cache")
+    if not sh.isdir(cache_path):
+        sh.mkdir(cache_path)
+    cmdline = [
+        PIP_EXECUTABLE, '-v',
+        'install', '-I', '-U',
+        '--download', download_dir,
+        '--build', build_path,
+        '--download-cache', cache_path,
+    ]
+    # Don't download wheels...
+    #
+    # See: https://github.com/pypa/pip/issues/1439
+    if dist_version.StrictVersion(PIP_VERSION) >= dist_version.StrictVersion('1.5'):
+        cmdline.append("--no-use-wheel")
+    cmdline.extend([str(p) for p in pips_to_download])
+    sh.execute_save_output(cmdline, output_filename)

anvil/utils.py

@@ -218,6 +218,33 @@ def wait_for_url(url, max_attempts=5,
     six.reraise(exc_type, exc, exc_tb)
 
 
+def retry(attempts, delay, func, *args, **kwargs):
+    if delay < 0:
+        raise ValueError("delay must be >= 0")
+    if attempts < 0:
+        raise ValueError("attempts must be >= 1")
+    func_name = "??"
+    try:
+        func_name = func.__name__
+    except AttributeError:
+        pass
+    failures = []
+    max_attempts = int(attempts) + 1
+    for attempt in range(1, max_attempts):
+        LOG.debug("Attempt %s for calling '%s'", attempt, func_name)
+        kwargs['attempt'] = attempt
+        try:
+            return func(*args, **kwargs)
+        except Exception:
+            failures.append(sys.exc_info())
+            if attempt < max_attempts and delay > 0:
+                LOG.info("Waiting %s seconds before calling '%s' again",
+                         delay, func_name)
+                sh.sleep(delay)
+    exc_type, exc, exc_tb = failures[-1]
+    six.reraise(exc_type, exc, exc_tb)
+
+
 def add_header(fn, contents, adjusted=True):
     lines = []
     if not fn: