launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/lp-signing:debug-subprocess-output into lp-signing:master.

Commit message:
When a subprocess command (used to sign messages) fails, it is not currently possible to see the output of that command. I'm proposing this to allow us to raise the same exception as subprocess.run raises on command failure, but only after we send to the debug log the output of the command.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/lp-signing/+git/lp-signing/+merge/381780
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/lp-signing:debug-subprocess-output into lp-signing:master.

diff --git a/lp_signing/model/key.py b/lp_signing/model/key.py
index 863660a..b47a7ac 100644
--- a/lp_signing/model/key.py
+++ b/lp_signing/model/key.py
@@ -9,23 +9,13 @@ __all__ = [
 
 from contextlib import contextmanager
 import logging
-from pathlib import Path
 import shutil
 import subprocess
+from subprocess import CalledProcessError
 from tempfile import TemporaryDirectory
 from textwrap import dedent
 
 from flask_storm import store
-import pytz
-from storm.locals import (
-    DateTime,
-    Int,
-    RawStr,
-    Reference,
-    Storm,
-    Unicode,
-    )
-
 from lp_signing.database.constants import DEFAULT
 from lp_signing.database.enumcol import DBEnum
 from lp_signing.enums import (
@@ -40,6 +30,16 @@ from lp_signing.exceptions import (
     UnsupportedSignatureMode,
     )
 from lp_signing.model.client import Client
+from pathlib import Path
+import pytz
+from storm.locals import (
+    DateTime,
+    Int,
+    RawStr,
+    Reference,
+    Storm,
+    Unicode,
+    )
 
 
 _log = logging.getLogger(__name__)
@@ -86,12 +86,20 @@ def _temporary_path():
 
 
 def _log_subprocess_run(args, **kwargs):
+    check = kwargs.pop('check', False)
     process = subprocess.run(
         args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, **kwargs)
     for line in process.stdout.splitlines():
         if isinstance(line, bytes):
             line = line.decode("UTF-8", errors="replace")
         _log.debug("%s: %s", args[0], line)
+
+    # Dealing with subprocess.run(check=True) here, so we have the
+    # opportunity to see the command output in a debug log above.
+    if check and process.returncode:
+        raise CalledProcessError(
+            process.returncode, process.args, output=process.stdout,
+            stderr=process.stdout)
     return process