PR#1315: Get text output from subprocess in SCM - fm-orchestrator

fm-orchestrator

#1315 Get text output from subprocess in SCM

Closed 4 years ago by cqi. Opened 4 years ago by cqi.

cqi/fm-orchestrator get-text-output-from-subprocess into master

Get text output from subprocess in SCM

Chenxiong Qi • 4 years ago

01992ee

module_build_service/scm.py

file modified

+4 -4

		`@@ -117,7 +117,7 @@`
		`branches = SCM._run(`
		`["git", "branch", "-r", "--contains", self.commit], chdir=self.sourcedir`
		`)[1]`
		`- for branch in branches.decode("utf-8").split("\n"):`
		`+ for branch in branches.split("\n"):`
		`branch = branch.strip()`
		`if branch[len("origin/"):] == self.branch:`
		`found = True`
		`@@ -137,7 +137,7 @@`

		`@staticmethod`
		`def _run_without_retry(cmd, chdir=None, log_stdout=False):`
		`- proc = sp.Popen(cmd, stdout=sp.PIPE, stderr=sp.PIPE, cwd=chdir)`
		`+ proc = sp.Popen(cmd, stdout=sp.PIPE, stderr=sp.PIPE, cwd=chdir, universal_newlines=True)`
mprahl commented 4 years ago What encoding ends up being used here? Does it make sense to pass in `encoding='utf-8'` (untested)?
cqi commented 4 years ago Python 3 documentation mentions that https://docs.python.org/3/library/subprocess.html#frequently-used-arguments It depends on `locale.getpreferredencoding`.
		`stdout, stderr = proc.communicate()`
		`if log_stdout and stdout:`
		`log.debug(stdout)`
		`@@ -225,7 +225,7 @@`
		`# 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")`
		`+ return output.split()[0]`
		`else:`
		`raise RuntimeError("get_latest: Unhandled SCM scheme.")`

		`@@ -270,7 +270,7 @@`
		`shutil.rmtree(td)`

		`if output:`
		`- return str(output.decode("utf-8").strip("\n"))`
		`+ return str(output.strip("\n"))`

		`raise UnprocessableEntity(`
		`'The full commit hash of "{0}" for "{1}" could not be found'.format(`

cqi commented 4 years ago

Some code benefits from this change that is no need to call decode method to
convert to text.

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

mprahl commented on line 15 of module_build_service/scm.py 4 years ago

What encoding ends up being used here? Does it make sense to pass in encoding='utf-8' (untested)?

cqi commented on line 15 of module_build_service/scm.py 4 years ago

Python 3 documentation mentions that https://docs.python.org/3/library/subprocess.html#frequently-used-arguments

It depends on locale.getpreferredencoding.

mprahl commented 4 years ago

Python 3 documentation mentions that https://docs.python.org/3/library/subprocess.html#frequently-used-arguments
It depends on locale.getpreferredencoding.

Could we hard-code UTF-8 then instead of relying on the computer's configured locale?

cqi commented 4 years ago

Could we hard-code UTF-8 then instead of relying on the computer's configured locale?

Argument encoding is only avialable since Python 3.6. So, if we would like to ensure the coding is utf-8 explicitly, current code just works well and no need to make this change. I'm going to close this PR.

Pull-Request has been closed by cqi

4 years ago