PR#1070: Refactor SCM.get_latest - fm-orchestrator

fm-orchestrator

#1070 Refactor SCM.get_latest

Merged 5 years ago by mprahl. Opened 5 years ago by cqi.

cqi/fm-orchestrator refactor-SCM.get_latest into master

Refactor SCM.get_latest

Chenxiong Qi • 5 years ago

b577b0f

module_build_service/scm.py

file modified

+10 -21

		`@@ -196,31 +196,20 @@`
		`ref = 'master'`
		`if self.scheme == "git":`
		`log.debug("Getting/verifying commit hash for %s" % self.repository)`
		`- # get all the branches on the remote`
		`- output = SCM._run(["git", "ls-remote", "--exit-code", self.repository])[1]`
		`- # pair branch names and their latest refs into a dict. The output of the above command`
		`- # is multiple lines of "bf028e573e7c18533d89c7873a411de92d4d913e refs/heads/master".`
		`- # So the dictionary ends up in the format of`
		`- # {"master": "bf028e573e7c18533d89c7873a411de92d4d913e"...}.`
		`- branches = {}`
		`- for branch_and_ref in output.decode('utf-8').strip().split("\n"):`
		`- # Only look at refs/heads/* and not refs/remotes/origin/*`
		`- if "refs/heads/" not in branch_and_ref:`
		`- continue`
		`- # This grabs the last bit of text after the last "/", which is the branch name`
		`- cur_branch = branch_and_ref.split("\t")[-1].split("refs/heads/")[-1]`
		`- # This grabs the text before the first tab, which is the commit hash`
		`- cur_ref = branch_and_ref.split("\t")[0]`
		`- branches[cur_branch] = cur_ref`
		`- # first check if the branch name is in the repo`
		`- if ref in branches:`
		`- return branches[ref]`
		`- # if the branch is not in the repo it may be a ref.`
		`- else:`
		`+ try:`
		`+ _, output, _ = SCM._run([`
		`+ "git", "ls-remote", "--exit-code", self.repository, 'refs/heads/' + ref`
		`+ ])`
		`+ except UnprocessableEntity:`
		`# The call below will either return the commit hash as is (if a full one was`
		`# provided) or the full commit hash (if a short hash was provided). If ref is not`
		`# a commit hash, then this will raise an exception.`
		`return self.get_full_commit_hash(commit_hash=ref)`
		`+ else:`
		`+ # git-ls-remote prints output like this, where the first commit`
		`+ # hash is what to return.`
		`+ # bf028e573e7c18533d89c7873a411de92d4d913e refs/heads/master`
		`+ return output.split()[0].decode('utf-8')`
		`else:`
		`raise RuntimeError("get_latest: Unhandled SCM scheme.")`

tests/test_scm.py

file modified

-13

		`@@ -25,7 +25,6 @@`
		`import tempfile`

		`import pytest`
		`- from mock import patch`

		`import module_build_service.scm`
		`from module_build_service.errors import ValidationError, UnprocessableEntity`
		`@@ -138,15 +137,3 @@`
		`scm = module_build_service.scm.SCM(repo_url)`
		`with pytest.raises(UnprocessableEntity):`
		`scm.get_latest('15481faa232d66589e660cc301179867fb00842c9')`
		`-`
		`- @patch.object(module_build_service.scm.SCM, '_run')`
		`- def test_get_latest_ignore_origin(self, mock_run):`
		`- output = b"""\`
		`- 58379ef7887cbc91b215bacd32430628c92bc869\tHEAD`
		`- 58379ef7887cbc91b215bacd32430628c92bc869\trefs/heads/master`
		`- 10a651f39911a07d85fe87fcfe91999545e44ae0\trefs/remotes/origin/master`
		`- """`
		`- mock_run.return_value = (0, output, '')`
		`- scm = module_build_service.scm.SCM(repo_url)`
		`- commit = scm.get_latest(None)`
		`- assert commit == '58379ef7887cbc91b215bacd32430628c92bc869'`

cqi commented 5 years ago

git-ls-remote accepts paramter refs to just return refs for specific
ones. This refactor uses this parameter to avoid handling full list of
refs from remote repository.

In original code, option --exit-code is passed to git-ls-remote, which
would cause command return exit code 2 when no ref is found from remote
repository. SCM._run raises an error if that happens, but the raised
error is not handled. This refactor catches this error to ensure the
original behavior happens, that is if a ref is not found, treat it as a
commit hash and call SCM.get_full_commit_hash.

Signed-off-by: Chenxiong Qi cqi@redhat.com

rebased onto 14b08c2ef1f4e21e30f7435e0a6fb5dc109d9928

5 years ago