[node-patches] Change in ovirt-node[master]: ui: Rework utils.process functions

fabiand at fedoraproject.org fabiand at fedoraproject.org
Mon Jan 28 17:34:51 UTC 2013


Fabian Deutsch has uploaded a new change for review.

Change subject: ui: Rework utils.process functions
......................................................................

ui: Rework utils.process functions

Change-Id: I908e6680f0be23d637735c9d3a0359b929fcb43b
Signed-off-by: Fabian Deutsch <fabiand at fedoraproject.org>
---
M scripts/tui/src/ovirt/node/app.py
M scripts/tui/src/ovirt/node/config/defaults.py
M scripts/tui/src/ovirt/node/config/network.py
M scripts/tui/src/ovirt/node/setup/engine_page.py
M scripts/tui/src/ovirt/node/ui/__init__.py
M scripts/tui/src/ovirt/node/ui/urwid_builder.py
M scripts/tui/src/ovirt/node/ui/widgets.py
M scripts/tui/src/ovirt/node/utils/__init__.py
M scripts/tui/src/ovirt/node/utils/network.py
M scripts/tui/src/ovirt/node/utils/process.py
M scripts/tui/src/ovirt/node/utils/security.py
M scripts/tui/src/ovirt/node/utils/system.py
M scripts/tui/src/ovirt/node/utils/virt.py
M scripts/tui/src/ovirt/node/valid.py
14 files changed, 76 insertions(+), 87 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/72/11472/1

diff --git a/scripts/tui/src/ovirt/node/app.py b/scripts/tui/src/ovirt/node/app.py
index 879c809..30fa39d 100644
--- a/scripts/tui/src/ovirt/node/app.py
+++ b/scripts/tui/src/ovirt/node/app.py
@@ -210,7 +210,8 @@
                                           (element, cb, action))
                         cb.callback = action
             if type(element) is ui.SaveButton:
-                # http://stackoverflow.com/questions/2731111/python-lambdas-and-variable-bindings
+                # http://stackoverflow.com/questions/2731111/
+                # python-lambdas-and-variable-bindings
                 toggle_disabled = lambda t, v, e=element: e.enabled(v)
                 plugin.on_valid.connect(toggle_disabled)
 
@@ -331,7 +332,7 @@
 
     def __drop_to_shell(self):
         with self.ui.suspended():
-            utils.process.system("reset ; bash")
+            utils.process.call("reset ; bash")
 
     def __check_terminal_size(self):
         cols, rows = self.ui.size()
diff --git a/scripts/tui/src/ovirt/node/config/defaults.py b/scripts/tui/src/ovirt/node/config/defaults.py
index 5f82091..7fa71ed 100644
--- a/scripts/tui/src/ovirt/node/config/defaults.py
+++ b/scripts/tui/src/ovirt/node/config/defaults.py
@@ -756,16 +756,16 @@
                     cmd = "service kdump propagate"
                 cmd += "2>&1"
 
-                success, stdout = utils.process.pipe(cmd)
+                try:
+                    utils.process.check_call(cmd)
 
-                if success:
                     ovirt_store_config(["/root/.ssh/kdump_id_rsa.pub",
                                         "/root/.ssh/kdump_id_rsa",
                                         "/root/.ssh/known_hosts",
                                         "/root/.ssh/config"])
-                else:
+                except utils.process.CalledProcessError as e:
                     self.logger.warning("Failed to activate KDump with " +
-                                        "SSH: %s" % stdout)
+                                        "SSH: %s" % e)
 
         class RemoveKdumpConfig(utils.Transaction.Element):
             title = "Removing kdump backup"
@@ -777,7 +777,7 @@
                 from ovirtnode.ovirtfunctions import remove_config
 
                 remove_config("/etc/kdump.conf")
-                utils.process.system("service kdump stop")
+                utils.process.call("service kdump stop")
                 open('/etc/kdump.conf', 'w').close()
 
                 self.backups.remove()
@@ -791,12 +791,14 @@
             def commit(self):
                 from ovirtnode.ovirtfunctions import unmount_config, \
                     ovirt_store_config
-                from ovirt.node.utils.process import system
 
-                if utils.process.system("service kdump restart") > 0:
+                try:
+                    utils.process.check_call("service kdump restart")
+                except utils.process.CalledProcessError as e:
+                    self.logger.info("Failure while restarting kdump: %s" % e)
                     unmount_config("/etc/kdump.conf")
                     self.backups.restore("/etc/kdump.conf")
-                    system("service kdump restart")
+                    utils.process.call("service kdump restart")
 
 #                    raise RuntimeError("KDump configuration failed, " +
 #                                       "location unreachable. Previous " +
diff --git a/scripts/tui/src/ovirt/node/config/network.py b/scripts/tui/src/ovirt/node/config/network.py
index 03a35d7..9e8cbe2 100644
--- a/scripts/tui/src/ovirt/node/config/network.py
+++ b/scripts/tui/src/ovirt/node/config/network.py
@@ -28,8 +28,6 @@
 """
 
 
-
-
 LOGGER = logging.getLogger(__name__)
 
 
@@ -115,7 +113,7 @@
     """Get or set DNS servers
 
     >>> import ovirt.node.utils.process as p
-    >>> _, stdout = p.pipe("egrep '^nameserver' /etc/resolv.conf | wc -l")
+    >>> stdout = p.pipe("egrep '^nameserver' /etc/resolv.conf | wc -l")
     >>> len(nameservers()) == int(stdout)
     True
     """
diff --git a/scripts/tui/src/ovirt/node/setup/engine_page.py b/scripts/tui/src/ovirt/node/setup/engine_page.py
index 9f03c7e..ddcf4dc 100644
--- a/scripts/tui/src/ovirt/node/setup/engine_page.py
+++ b/scripts/tui/src/ovirt/node/setup/engine_page.py
@@ -161,7 +161,7 @@
         """
         cfg = dict(self.vdsm.retrieve())
         server = cfg["server"]
-        retval = utils.process.system("ping -c 1 '%s'" % server)
+        retval = utils.process.call("ping -c 1 '%s'" % server)
         self.logger.debug("Pinged server with %s" % retval)
         if retval != 0:
             raise RuntimeError("Unable to reach given server: %s" %
diff --git a/scripts/tui/src/ovirt/node/ui/__init__.py b/scripts/tui/src/ovirt/node/ui/__init__.py
index 7d6c6fc..58a0dbb 100644
--- a/scripts/tui/src/ovirt/node/ui/__init__.py
+++ b/scripts/tui/src/ovirt/node/ui/__init__.py
@@ -170,6 +170,7 @@
         dialog: The dialog to close
     """
     dialog = None
+
     def __init__(self, callback=None, dialog=None):
         super(CloseAction, self).__init__(callback)
         self.dialog = dialog
diff --git a/scripts/tui/src/ovirt/node/ui/urwid_builder.py b/scripts/tui/src/ovirt/node/ui/urwid_builder.py
index 9c0bd7e..c8ed4f3 100644
--- a/scripts/tui/src/ovirt/node/ui/urwid_builder.py
+++ b/scripts/tui/src/ovirt/node/ui/urwid_builder.py
@@ -476,7 +476,7 @@
             # Hack to alow to close a dialog by name
             for d in self.__widget_stack:
                 if d.title == dialog:
-                    dialog = d 
+                    dialog = d
         self.logger.debug("Widget stack: %s" % self.__widget_stack)
         new_stack = [w for w in self.__widget_stack if w != dialog]
         self.__widget_stack = new_stack
diff --git a/scripts/tui/src/ovirt/node/ui/widgets.py b/scripts/tui/src/ovirt/node/ui/widgets.py
index c8f71b6..82825f0 100644
--- a/scripts/tui/src/ovirt/node/ui/widgets.py
+++ b/scripts/tui/src/ovirt/node/ui/widgets.py
@@ -102,7 +102,6 @@
             urwid.emit_signal(self, "changed", widget)
         urwid.connect_signal(self.__walker, 'modified', __on_item_change)
 
-
         self.__box = urwid.BoxAdapter(self.__list, height)
         self.__box_attrmap = urwid.AttrMap(self.__box, self._table_attr)
 
diff --git a/scripts/tui/src/ovirt/node/utils/__init__.py b/scripts/tui/src/ovirt/node/utils/__init__.py
index 06c7375..7603c09 100644
--- a/scripts/tui/src/ovirt/node/utils/__init__.py
+++ b/scripts/tui/src/ovirt/node/utils/__init__.py
@@ -20,7 +20,6 @@
 # also available at http://www.gnu.org/copyleft/gpl.html.
 from ovirt.node import base, exceptions
 import augeas as _augeas
-import hashlib
 import system_config_keyboard.keyboard
 import time
 import traceback
@@ -159,6 +158,7 @@
     def get_current(self):
         return self.kbd.get()
 
+
 class Transaction(list, base.Base):
     """A very simple transaction mechanism.
 
diff --git a/scripts/tui/src/ovirt/node/utils/network.py b/scripts/tui/src/ovirt/node/utils/network.py
index 89a4692..9da1051 100644
--- a/scripts/tui/src/ovirt/node/utils/network.py
+++ b/scripts/tui/src/ovirt/node/utils/network.py
@@ -304,7 +304,6 @@
     def exists(self):
         """If this NIC currently exists in the system
         """
-        
         return self.iface in all_ifaces()
 
     def has_link(self):
@@ -316,7 +315,7 @@
         >>> iface = all_ifaces()[0]
         >>> cmd = "ip link set dev {dev} up ;"
         >>> cmd += "ip link show {dev}".format(dev=iface)
-        >>> has_carrier = "LOWER_UP" in process.pipe(cmd, without_retval=True)
+        >>> has_carrier = "LOWER_UP" in process.pipe(cmd)
         >>> has_carrier == NIC(iface).has_link()
         True
 
@@ -387,8 +386,7 @@
 
         # Fallback
         cmd = "ip -o addr show {iface}".format(iface=self.iface)
-        stdout = str(process.pipe(cmd, without_retval=True))
-        for line in stdout.split("\n"):
+        for line in process.pipe(cmd).split("\n"):
             token = re.split("\s+", line)
             if re.search("\sinet[6]?\s", line):
                 addr, mask = token[3].split("/")
@@ -429,8 +427,7 @@
         # Fallback
         gw = None
         cmd = "ip route list"
-        stdout = str(process.pipe(cmd, without_retval=True))
-        for line in stdout.split("\n"):
+        for line in process.pipe(cmd).split("\n"):
             token = re.split("\s+", line)
             if line.startswith("default via"):
                 gw = token[2]
@@ -497,6 +494,5 @@
         The current hostname
     """
     if new_hostname:
-        utils.process.system("hostname %s" % new_hostname)
-    stdout = unicode(utils.process.pipe("hostname", without_retval=True))
-    return stdout.strip()
+        utils.process.call("hostname %s" % new_hostname)
+    return utils.process.pipe("hostname").strip()
diff --git a/scripts/tui/src/ovirt/node/utils/process.py b/scripts/tui/src/ovirt/node/utils/process.py
index 3eb9c50..37935e4 100644
--- a/scripts/tui/src/ovirt/node/utils/process.py
+++ b/scripts/tui/src/ovirt/node/utils/process.py
@@ -28,65 +28,59 @@
 
 LOGGER = logging.getLogger(__name__)
 
+COMMON_POPEN_ARGS = {
+    "close_fds": True
+}
+
+CalledProcessError = subprocess.CalledProcessError
+
 
 def popen(*args, **kwargs):
     """subprocess.Popen wrapper to not leak file descriptors
-
-    Args:
-        cmd: Cmdline to be run
-
-    Returns:
-        Popen object
     """
-    kwargs.update({
-        "close_fds": True
-    })
+    kwargs.update(COMMON_POPEN_ARGS)
+    LOGGER.debug("Popen with: %s %s" % (args, kwargs))
     return subprocess.Popen(*args, **kwargs)
 
 
-def system(cmd):
-    """Run a non-interactive command, or where the user shall input something
+def call(*args, **kwargs):
+    """subprocess.call wrapper to not leak file descriptors
+    """
+    kwargs.update(COMMON_POPEN_ARGS)
+    LOGGER.debug("Calling with: %s %s" % (args, kwargs))
+    return int(subprocess.check_call(*args, **kwargs))
+
+
+def check_call(*args, **kwargs):
+    """subprocess.check_call wrapper to not leak file descriptors
+    """
+    kwargs.update(COMMON_POPEN_ARGS)
+    LOGGER.debug("Checking call with: %s %s" % (args, kwargs))
+    return int(subprocess.check_call(*args, **kwargs))
+
+
+def check_output(*args, **kwargs):
+    """subprocess.check_output wrapper to not leak file descriptors
+    """
+    kwargs.update(COMMON_POPEN_ARGS)
+    LOGGER.debug("Checking output with: %s %s" % (args, kwargs))
+    return unicode(subprocess.check_output(*args, **kwargs))
+
+
+def pipe(cmd, stdin=None):
+    """Run a non-interactive command and return it's output
 
     Args:
         cmd: Cmdline to be run
+        stdin: (optional) Data passed to stdin
 
     Returns:
-        retval of the process
+        stdout, stderr of the process (as one blob)
     """
-    return popen(cmd, shell=True).wait()
-
-
-def pipe(cmd, stdin=None, without_retval=False):
-    """Run a command interactively and catch it's output.
-    This functions allows to pass some input to a running command.
-
-    >>> r = pipe("echo -n Hi")
-    >>> type(r[1])
-    <type 'str'>
-
-    >>> r
-    (True, 'Hi')
-
-    Args:
-        cmd: Commandline to be run
-        stdin: Optional string passed as stdin
-        without_retval: Optional if no retval should be passed
-
-    Returns:
-        A tuple (success, stdout)
-    """
-
-    LOGGER.debug("run '%s'" % cmd)
-    system_cmd = popen(cmd, shell=True, stdout=subprocess.PIPE,
-                       stderr=subprocess.PIPE)
-    stdout, stderr = system_cmd.communicate(stdin)
-    if stdout:
-        LOGGER.debug("out '%s'" % stdout)
-    if stderr:
-        LOGGER.warning("error '%s'" % stderr)
-    if without_retval:
-        return stdout
-    return (system_cmd.returncode == 0, stdout)
+    return unicode(popen(cmd, shell=True,
+                         stdin=subprocess.PIPE,
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.STDOUT).communicate(stdin)[0])
 
 
 def pipe_async(cmd, stdin=None):
@@ -101,7 +95,7 @@
         Lines read from stdout
     """
     # https://github.com/wardi/urwid/blob/master/examples/subproc.py
-    LOGGER.debug("run async '%s'" % cmd)
+    LOGGER.debug("Piping async '%s'" % cmd)
     process = popen(cmd, shell=True, stdout=subprocess.PIPE,
                     stderr=subprocess.PIPE, stdin=stdin)
     if stdin:
diff --git a/scripts/tui/src/ovirt/node/utils/security.py b/scripts/tui/src/ovirt/node/utils/security.py
index 9563422..06b055f 100644
--- a/scripts/tui/src/ovirt/node/utils/security.py
+++ b/scripts/tui/src/ovirt/node/utils/security.py
@@ -21,7 +21,7 @@
 from ovirt.node import base, valid, utils
 import process
 import os.path
-import PAM as _PAM
+import PAM as _PAM  # @UnresolvedImport
 
 """
 Some convenience functions related to security
@@ -43,11 +43,11 @@
         additional_lines = []
         ofunc.unmount_config("/etc/profile")
 
-        process.system("sed -i '/OPENSSL_DISABLE_AES_NI/d' /etc/profile")
+        process.check_call("sed -i '/OPENSSL_DISABLE_AES_NI/d' /etc/profile")
         if disable_aes:
             additional_lines += ["export OPENSSL_DISABLE_AES_NI=1"]
 
-        process.system("sed -i '/SSH_USE_STRONG_RNG/d' /etc/profile")
+        process.check_call("sed -i '/SSH_USE_STRONG_RNG/d' /etc/profile")
         if rng_num_bytes:
             additional_lines += ["export SSH_USE_STRONG_RNG=%s" %
                                  rng_num_bytes]
@@ -91,7 +91,7 @@
 
     def restart(self):
         self.logger.debug("Restarting SSH")
-        process.system("service sshd restart &>/dev/null")
+        process.call("service sshd restart &>/dev/null")
 
     def password_authentication(self, enable=None):
         augpath = "/files/etc/ssh/sshd_config/PasswordAuthentication"
@@ -115,8 +115,7 @@
             hostkey = hkf.read()
 
         hostkey_fp_cmd = "ssh-keygen -l -f '%s'" % fn_hostkey
-        stdout = process.pipe(hostkey_fp_cmd, without_retval=True)
-        fingerprint = stdout.strip().split(" ")[1]
+        fingerprint = process.pipe(hostkey_fp_cmd).strip().split(" ")[1]
         return (fingerprint, hostkey)
 
 
diff --git a/scripts/tui/src/ovirt/node/utils/system.py b/scripts/tui/src/ovirt/node/utils/system.py
index d8b2cb1..b8b59a2 100644
--- a/scripts/tui/src/ovirt/node/utils/system.py
+++ b/scripts/tui/src/ovirt/node/utils/system.py
@@ -29,13 +29,12 @@
 """
 
 
-
 def reboot():
-    process.system("reboot")
+    process.call("reboot")
 
 
 def poweroff():
-    process.system("poweroff")
+    process.call("poweroff")
 
 
 def is_efi():
diff --git a/scripts/tui/src/ovirt/node/utils/virt.py b/scripts/tui/src/ovirt/node/utils/virt.py
index 7414720..b43f329 100644
--- a/scripts/tui/src/ovirt/node/utils/virt.py
+++ b/scripts/tui/src/ovirt/node/utils/virt.py
@@ -87,7 +87,7 @@
     try:
         with LibvirtConnection() as con:
             num_domains = str(con.numOfDomains())
-    except libvirt.libvirtError as e:
+    except libvirt.libvirtError:
         pass
         # warning("Error while working with libvirt: %s" % e.message)
     return num_domains
diff --git a/scripts/tui/src/ovirt/node/valid.py b/scripts/tui/src/ovirt/node/valid.py
index 706d3fa..eb995ec 100644
--- a/scripts/tui/src/ovirt/node/valid.py
+++ b/scripts/tui/src/ovirt/node/valid.py
@@ -31,7 +31,6 @@
 """
 
 
-
 class Validator(base.Base):
     """This class is used to validate user inputs
     Basically an exception is raised if an invalid value was given. The value
@@ -215,7 +214,8 @@
             self.description = "%s" % (exactly)
 
     def validate(self, value):
-        self.logger.debug("Checking number %s %s %s" % (self, self.pattern, value))
+        self.logger.debug("Checking number %s %s %s" % (self, self.pattern,
+                                                        value))
         valid = RegexValidator.validate(self, value)
         if valid and self.bounds:
             self.logger.debug("Checking bounds: %s" % self.bounds)
@@ -442,6 +442,6 @@
                 subprocess.check_call("test -b %s" % value, shell=True,
                                       close_fds=True)
                 is_valid = True
-        except Exception as e:
+        except Exception:
             is_valid = False
-        return is_valid
\ No newline at end of file
+        return is_valid


--
To view, visit http://gerrit.ovirt.org/11472
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I908e6680f0be23d637735c9d3a0359b929fcb43b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Fabian Deutsch <fabiand at fedoraproject.org>



More information about the node-patches mailing list