2016-07-19 10:11 GMT+02:00 Jan Tluka <jtluka@redhat.com>:
Mon, Jul 18, 2016 at 06:44:07PM CEST, jprochaz@redhat.com wrote:
>Method check_output used for getting hash of HEAD in git repo was
>introduced in Python2.7 and it does not work for Python2.6 which is
>default Python version for RHEL6.x.
>
>This patch replaces method check_output with Popen which is available in
>both 2.6 and 2.7.
>
>Fixes #174
>
>Signed-off-by: Jiri Prochazka <jprochaz@redhat.com>
>---
> lnst/Common/Config.py | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
>diff --git a/lnst/Common/Config.py b/lnst/Common/Config.py
>index 5362da1..29c2f16 100644
>--- a/lnst/Common/Config.py
>+++ b/lnst/Common/Config.py
>@@ -34,19 +34,20 @@ class Config():
>
>     def _get_version(self):
>         # Check if I'm in git
>-        try:
>-            cwd = os.getcwd()
>-            abspath = os.path.abspath(__file__)
>-            dname = os.path.dirname(abspath)
>-            os.chdir(dname)
>-            with open(os.devnull, 'w') as null:
>-                head = subprocess.check_output(['git', 'rev-parse', 'HEAD'],
>-                                               stderr=null).strip()
>-            return head
>-        except subprocess.CalledProcessError:
>-            return LNSTMajorVersion
>-        finally:
>-            os.chdir(cwd)
>+        cwd = os.getcwd()
>+        abspath = os.path.abspath(__file__)
>+        dname = os.path.dirname(abspath)
>+        os.chdir(dname)
>+        with open(os.devnull, 'w') as null:
>+            proc = subprocess.Popen(['git', 'rev-parse', 'HEAD'],
>+                                           stdout=subprocess.PIPE,
>+                                           stderr=null).communicate()
                                                         ^^^^
That looks ugly and is hard to read!

How about exception handling?
https://docs.python.org/2.6/library/subprocess.html#exceptions


​Unlike check_output, neither Popen constructor or communicate() method throws exception when execution of the command failed.



>+            if proc[0] != '':
>+                version = proc[0].strip()
>+            else:
>+                version = LNSTMajorVersion
>+        os.chdir(cwd)
>+        return version
>
>     def controller_init(self):
>         self._options['environment'] = dict()
>--
>2.4.11
>_______________________________________________
>LNST-developers mailing list
>lnst-developers@lists.fedorahosted.org
>https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahosted.org