PR#1139: Do not retry on 'git ls-remote' failure, but instead fallback to 'git clone'. - fm-orchestrator

fm-orchestrator

#1139 Do not retry on 'git ls-remote' failure, but instead fallback to 'git clone'.

Merged 5 years ago by lucarval. Opened 5 years ago by jkaluza.

jkaluza/fm-orchestrator ls-remote into master

Do not retry on 'git ls-remote' failure, but instead fallback to 'git clone'.

Jan Kaluza • 5 years ago

130911b

module_build_service/scm.py

file modified

+16 -6

		`@@ -129,11 +129,7 @@`
		`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 @@`
		`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 @@`
		`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:`

tests/test_scm.py

file modified

		`@@ -57,6 +57,13 @@`
		`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.`

jkaluza commented 5 years ago

This blocks https://pagure.io/fedora-infrastructure/issue/7550.

The reason is that in case you use "commit hash" in modules, the ls-remote will always fail and the retry just blocks for 15 seconds before trying again. Instead, we can just fallback to git clone which works with both commit hashes and branch names and do possible retry there.