From 130911bb490de87ba30e74144f54d26234997e08 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Feb 11 2019 14:27:07 +0000 Subject: Do not retry on 'git ls-remote' failure, but instead fallback to 'git clone'. --- diff --git a/module_build_service/scm.py b/module_build_service/scm.py index 0f8917d..7e7e43b 100644 --- a/module_build_service/scm.py +++ b/module_build_service/scm.py @@ -129,11 +129,7 @@ class SCM(object): return None @staticmethod - @retry( - timeout=conf.scm_net_timeout, - interval=conf.scm_net_retry_interval, - wait_on=UnprocessableEntity) - def _run(cmd, chdir=None, log_stdout=False): + def _run_without_retry(cmd, chdir=None, log_stdout=False): proc = sp.Popen(cmd, stdout=sp.PIPE, stderr=sp.PIPE, cwd=chdir) stdout, stderr = proc.communicate() if log_stdout and stdout: @@ -145,6 +141,14 @@ class SCM(object): cmd, proc.returncode, stdout, stderr)) return proc.returncode, stdout, stderr + @staticmethod + @retry( + timeout=conf.scm_net_timeout, + interval=conf.scm_net_retry_interval, + wait_on=UnprocessableEntity) + def _run(cmd, chdir=None, log_stdout=False): + return SCM._run_without_retry(cmd, chdir, log_stdout) + def checkout(self, scmdir): """Checkout the module from SCM. @@ -198,7 +202,13 @@ class SCM(object): if self.scheme == "git": log.debug("Getting/verifying commit hash for %s" % self.repository) try: - _, output, _ = SCM._run([ + # This will fail if `ref` is not a branch name, but for example commit hash. + # It is valid use-case to use commit hashes in modules instead of branch names + # and in this case, there is no other way to get the full commit hash then + # fallbac to `get_full_commit_hash`. We do not want to retry here, because + # in case module contains only commit hashes, it would block for very long + # time. + _, output, _ = SCM._run_without_retry([ "git", "ls-remote", "--exit-code", self.repository, 'refs/heads/' + ref ]) except UnprocessableEntity: diff --git a/tests/test_scm.py b/tests/test_scm.py index e272c63..8fd83f1 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -57,6 +57,13 @@ class TestSCMModule: target = '5481faa232d66589e660cc301179867fb00842c9' assert latest == target, "%r != %r" % (latest, target) + def test_local_get_latest_commit_hash_is_sane(self): + """ See that a hash is returned by scm.get_latest. """ + scm = module_build_service.scm.SCM(repo_url) + latest = scm.get_latest('5481f') + target = '5481faa232d66589e660cc301179867fb00842c9' + assert latest == target, "%r != %r" % (latest, target) + def test_local_get_latest_unclean_input(self): """ Ensure that shell characters aren't handled poorly.