Merge lp:~fo0bar/turku/turku-storage-log-output into lp:turku

[Ryan Finnie]

Log run output directly to logger

Python 2.7 made real-time logging of subprocess piped output difficult
(and actually a bit dangerous). But now that Turku is Python 3, Popen's
context manager will DTRT and clean up after itself, so we don't need an
intermediary.

Note that this removes return_output from run_logging(), but
return_output was not actually used.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

Looks good. I hadn't noticed that Popen objects could be used as context managers, but see it documented since 3.2 and implementing behavior we need.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 39

=== modified file 'turku_storage/ping.py'
--- turku_storage/ping.py	2020-06-21 22:41:21 +0000
+++ turku_storage/ping.py	2020-06-21 22:41:21 +0000
@@ -60,28 +60,21 @@
         self.lh_master.setLevel(logging.DEBUG)
         self.logger.addHandler(self.lh_master)
 
-    def run_logging(
-        self, args, loglevel=logging.DEBUG, cwd=None, env=None, return_output=False
-    ):
+    def run_logging(self, args, loglevel=logging.DEBUG, cwd=None, env=None):
         self.logger.log(loglevel, "Running: %s" % repr(args))
-        t = tempfile.NamedTemporaryFile(mode="w+", encoding="UTF-8")
-        self.logger.log(
-            loglevel, "(Command output is in %s until written here at the end)" % t.name
-        )
-        returncode = subprocess.call(args, cwd=cwd, env=env, stdout=t, stderr=t)
-        t.flush()
-        t.seek(0)
-        out = ""
-        for line in t.readlines():
-            if return_output:
-                out = out + line
-            self.logger.log(loglevel, line.rstrip("\n"))
-        t.close()
-        self.logger.log(loglevel, "Return code: %d" % returncode)
-        if return_output:
-            return (returncode, out)
-        else:
-            return returncode
+        with subprocess.Popen(
+            args,
+            cwd=cwd,
+            env=env,
+            encoding="UTF-8",
+            stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT,
+        ) as proc:
+            with proc.stdout as stdout:
+                for line in iter(stdout.readline, ""):
+                    self.logger.log(loglevel, line.rstrip("\n"))
+        self.logger.log(loglevel, "Return code: %d" % proc.returncode)
+        return proc.returncode
 
     def process_ping(self):
         jsonin = ""