[node-patches] Change in ovirt-node[master]: Code cleanup

rbarry at redhat.com rbarry at redhat.com
Mon Jun 23 21:54:03 UTC 2014


Ryan Barry has uploaded a new change for review.

Change subject: Code cleanup
......................................................................

Code cleanup

pep8 keeps getting stricter...

Change-Id: I910bddc6ac82e362863d60e22b5ffc5ad922f77a
Signed-off-by: Ryan Barry <rbarry at redhat.com>
---
M src/Makefile.am
M src/Makefile.check
M src/ovirt/node/base.py
M src/ovirt/node/config/defaults.py
M src/ovirt/node/config/migrate.py
M src/ovirt/node/installer/core/boot_device_page.py
M src/ovirt/node/setup/cim/cim_model.py
M src/ovirt/node/setup/core/diagnostics_page.py
M src/ovirt/node/setup/core/network_page.py
M src/ovirt/node/setup/core/plugins_page.py
M src/ovirt/node/setup/puppet/puppet_page.py
M src/ovirt/node/setup/snmp/snmp_model.py
M src/ovirt/node/ui/__init__.py
M src/ovirt/node/ui/urwid_builder.py
M src/ovirt/node/ui/widgets.py
M src/ovirt/node/utils/__init__.py
M src/ovirt/node/utils/expose.py
M src/ovirt/node/utils/fs.py
M src/ovirt/node/utils/network.py
M src/ovirt/node/utils/storage.py
M src/ovirt/node/utils/system.py
M src/ovirt/node/utils/virt.py
M src/ovirt/node/valid.py
23 files changed, 46 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/94/29094/1

diff --git a/src/Makefile.am b/src/Makefile.am
index 959ac23..02d9b61 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -56,6 +56,7 @@
 ovirt_node_locale_endir = /usr/share/locale/en_US/LC_MESSAGES
 pyovirt_node_uidir = $(pyovirt_nodedir)/ui
 pyovirt_node_utilsdir = $(pyovirt_nodedir)/utils
+pyovirt_node_utils_fsdir = $(pyovirt_nodedir)/utils
 pyovirt_node_toolsdir = $(pyovirt_nodedir)/tools
 
 # Path to installer app and installer plugins
diff --git a/src/Makefile.check b/src/Makefile.check
index dd14399..c208d36 100644
--- a/src/Makefile.check
+++ b/src/Makefile.check
@@ -22,7 +22,7 @@
 	@echo Passed $@
 
 %.doctest:
-	grep -q ">>>" "$*" && python -m doctest "$*" || :
+	python test.py $*
 
 check-static-pep8: $(PYTHONSOURCES:%=%.pep8)
 	@echo Passed $@
diff --git a/src/ovirt/node/base.py b/src/ovirt/node/base.py
index 4d1bb34..31c04c9 100644
--- a/src/ovirt/node/base.py
+++ b/src/ovirt/node/base.py
@@ -74,7 +74,7 @@
         def emit(self, userdata=None):
             """Emit a signal
             """
-            #self.logger.debug("%s: %s" % (self, self.callbacks))
+            # self.logger.debug("%s: %s" % (self, self.callbacks))
             for idx, cb in enumerate(self.callbacks):
                 self.logger.debug("%s (%d/%d) %s" %
                                   (self, idx + 1, len(self.callbacks), cb))
@@ -84,7 +84,7 @@
             return self
 
         def connect(self, cb):
-            #self.logger.debug("Connecting %s with %s" % (self, cb))
+            # self.logger.debug("Connecting %s with %s" % (self, cb))
             self.callbacks.append(cb)
             return self
 
diff --git a/src/ovirt/node/config/defaults.py b/src/ovirt/node/config/defaults.py
index 42d1a0f..e6298b5 100644
--- a/src/ovirt/node/config/defaults.py
+++ b/src/ovirt/node/config/defaults.py
@@ -243,8 +243,8 @@
     def configure_no_networking(self, iface=None):
         """Can be used to disable all networking
         """
-        #iface = iface or self.retrieve()["iface"]
-        #name = iface + "-DISABLED"
+        # iface = iface or self.retrieve()["iface"]
+        # name = iface + "-DISABLED"
         # FIXME why should we use ifname-DISABLED here?
         self.update(None, None, None, None, None, None)
 
diff --git a/src/ovirt/node/config/migrate.py b/src/ovirt/node/config/migrate.py
index 6267109..026e822 100644
--- a/src/ovirt/node/config/migrate.py
+++ b/src/ovirt/node/config/migrate.py
@@ -168,12 +168,12 @@
             SAM_REG_ADDR = "subscription.rhn.redhat.com"
             CANDLEPIN_CERT_FILE = "/etc/rhsm/ca/candlepin-local.pem"
 
-            if not RHN_XMLRPC_ADDR in rhn_conf["serverURL"] and not \
+            if RHN_XMLRPC_ADDR not in rhn_conf["serverURL"] and not \
                     rhn.sam_check():
                 rhn_url = rhn_conf["serverURL"]
                 rhn_ca = rhn_conf["sslCACert"]
             elif rhn.sam_check():
-                if not SAM_REG_ADDR in rhn_conf["hostname"]:
+                if SAM_REG_ADDR not in rhn_conf["hostname"]:
                     rhn_url = "https://%s" % rhn_conf["hostname"]
                     if os.path.exists(CANDLEPIN_CERT_FILE):
                         rhn_ca = CANDLEPIN_CERT_FILE
diff --git a/src/ovirt/node/installer/core/boot_device_page.py b/src/ovirt/node/installer/core/boot_device_page.py
index cd5a8f6..8a26aaa 100644
--- a/src/ovirt/node/installer/core/boot_device_page.py
+++ b/src/ovirt/node/installer/core/boot_device_page.py
@@ -89,14 +89,20 @@
 
     def on_change(self, changes):
         self.logger.debug("Boot device changes: %s" % changes)
-        if changes.contains_any(["boot.device"]):
-            device = changes["boot.device"]
-            if device == "other":
-                self.widgets["label.details"].text("")
-            else:
-                changes["boot.device.current"] = device
-                self._model.update(changes)
-                self.widgets["label.details"].set_device(device)
+        devices = self.storage_discovery.all_devices_for_ui_table()
+        if devices:
+            if changes.contains_any(["boot.device"]):
+                device = changes["boot.device"]
+                if device == "other":
+                    self.widgets["label.details"].text("")
+                else:
+                    changes["boot.device.current"] = device
+                    self._model.update(changes)
+                    self.widgets["label.details"].set_device(device)
+        else:
+            # Disable the continue button
+            buttons = plugins.UIElements(self.widgets["boot"].buttons)
+            buttons["button.next"].enabled(False)
 
         if changes.contains_any(["boot.device.custom"]):
             if self.storage_discovery.devices.live_disk_name() == \
diff --git a/src/ovirt/node/setup/cim/cim_model.py b/src/ovirt/node/setup/cim/cim_model.py
index 4f590d6..eaa4dbd 100644
--- a/src/ovirt/node/setup/cim/cim_model.py
+++ b/src/ovirt/node/setup/cim/cim_model.py
@@ -33,7 +33,7 @@
 
     >>> from ovirt.node.utils import fs
     >>> n = CIM(fs.FakeFs.File("dst"))
-    >>> n.update(True)
+    >>> _ = n.update(True)
     >>> n.retrieve()
     {'enabled': True}
     """
diff --git a/src/ovirt/node/setup/core/diagnostics_page.py b/src/ovirt/node/setup/core/diagnostics_page.py
index d34949f..9f0ee5f 100644
--- a/src/ovirt/node/setup/core/diagnostics_page.py
+++ b/src/ovirt/node/setup/core/diagnostics_page.py
@@ -1,4 +1,3 @@
-# -*- coding: utf-8 *-*
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
diff --git a/src/ovirt/node/setup/core/network_page.py b/src/ovirt/node/setup/core/network_page.py
index 9bb11d0..ba63cb5 100644
--- a/src/ovirt/node/setup/core/network_page.py
+++ b/src/ovirt/node/setup/core/network_page.py
@@ -96,7 +96,7 @@
     def __init__(self, app):
         super(Plugin, self).__init__(app)
 
-            # Keys/Paths to widgets related to NIC settings
+        # Keys/Paths to widgets related to NIC settings
         self._nic_details_group = self.widgets.group([
             "dialog.nic.ipv4.bootproto", "dialog.nic.ipv4.address",
             "dialog.nic.ipv4.netmask", "dialog.nic.ipv4.gateway",
@@ -153,7 +153,7 @@
         valid_bond_name = valid.RegexValidator("^(bond[0-9]{1,2}|007)$",
                                                "a valid bond name (bond[0-99])"
                                                )
-                                               # No regex, but for users ^
+        # No regex, but for users ^
 
         return {"hostname": fqdn_ip_or_empty,
                 "dns[0]": ip_or_empty,
@@ -458,7 +458,7 @@
         # Therefor we don't add it, to not call it twice.
         # But it should be added to the ocmplete transaction when the backend
         # code is more fine granular.
-        #txs += ipv6model.transaction()
+        # txs += ipv6model.transaction()
         return txs
 
 
diff --git a/src/ovirt/node/setup/core/plugins_page.py b/src/ovirt/node/setup/core/plugins_page.py
index a28c39f..075a115 100644
--- a/src/ovirt/node/setup/core/plugins_page.py
+++ b/src/ovirt/node/setup/core/plugins_page.py
@@ -147,7 +147,7 @@
         if re.compile(r'Name:.*?\nVer.*:.*?\nInstall Date:.*',
                       re.M | re.S).match(open(plugin_dir + f
                                               ).read()):
-            #Hopefully a plugin metadata file
+            # Hopefully a plugin metadata file
             with open(plugin_dir + f) as p:
                 lines = p.readlines()
             name = lines[0].strip().split(":")[1]
diff --git a/src/ovirt/node/setup/puppet/puppet_page.py b/src/ovirt/node/setup/puppet/puppet_page.py
index 57ae961..0cff470 100644
--- a/src/ovirt/node/setup/puppet/puppet_page.py
+++ b/src/ovirt/node/setup/puppet/puppet_page.py
@@ -108,8 +108,8 @@
     """Class to handle Puppet configuration in /etc/default/ovirt file
 
     >>> n = Puppet(fs.FakeFs.File("dst"))
-    >>> n.update(True, "puppet.example.com",
-    ...          "http://localhost/path/to/cert")
+    >>> _ = n.update(True, "puppet.example.com",
+    ...              "http://localhost/path/to/cert")
     >>> data = sorted(n.retrieve().items())
     >>> data[:2]
     [('certname', 'http://localhost/path/to/cert'), ('enabled', 'yes')]
diff --git a/src/ovirt/node/setup/snmp/snmp_model.py b/src/ovirt/node/setup/snmp/snmp_model.py
index a6999b6..5a611c5 100644
--- a/src/ovirt/node/setup/snmp/snmp_model.py
+++ b/src/ovirt/node/setup/snmp/snmp_model.py
@@ -81,7 +81,7 @@
 
     >>> from ovirt.node.utils import fs
     >>> n = SNMP(fs.FakeFs.File("dst"))
-    >>> n.update(True)
+    >>> _ = n.update(True)
     >>> n.retrieve()
     {'enabled': True}
     """
diff --git a/src/ovirt/node/ui/__init__.py b/src/ovirt/node/ui/__init__.py
index 9a0031f..ce1c94a 100644
--- a/src/ovirt/node/ui/__init__.py
+++ b/src/ovirt/node/ui/__init__.py
@@ -579,7 +579,7 @@
         super(Table, self).__init__(path, label, enabled)
         self.header = header
         if type(items) in [str, tuple, unicode]:
-            #For convenience, create a list of tuples if it is not already one
+            # For convenience, create a list of tuples if it is not already one
             if type(items) in [str, unicode]:
                 self.items = [(i, item) for i, item in zip(range(len(
                                                            items.split('\n'))),
diff --git a/src/ovirt/node/ui/urwid_builder.py b/src/ovirt/node/ui/urwid_builder.py
index 152dedf..973490c 100644
--- a/src/ovirt/node/ui/urwid_builder.py
+++ b/src/ovirt/node/ui/urwid_builder.py
@@ -119,9 +119,9 @@
         return widget
 
     def _build_button_bar(self, ui_buttonbar):
-        #if type(ui_buttonbar) is list:
+        # if type(ui_buttonbar) is list:
         children = ui_buttonbar
-        #else:
+        # else:
         #    children = ui_buttonbar.children
         # FIXME create dedicated widget
         button_widgets = []
diff --git a/src/ovirt/node/ui/widgets.py b/src/ovirt/node/ui/widgets.py
index 67e84a5..63c7b8d 100644
--- a/src/ovirt/node/ui/widgets.py
+++ b/src/ovirt/node/ui/widgets.py
@@ -234,8 +234,8 @@
                 self.knob_height = knob_height if knob_height > 1 else 1
                 if rows - 2 - self.knob_height > 0:
                     if self.knob_height is 1:
-                    #Calculate ourselves, subtracting 3 (for the arrows
-                    # and the knob itself)
+                        # Calculate ourselves, subtracting 3 (for the arrows
+                        # and the knob itself)
                         self.below_knob = abs(float(list_size - rows) / (rows -
                                                                          3))
                     else:
@@ -674,8 +674,6 @@
     save_button = None
 
     def __init__(self, widgets, title=None):
-#        self._listwalker = urwid.SimpleListWalker(widgets)
-#        self._container = urwid.ListBox(self._listwalker)
         self._container = TabablePile(widgets)
         self._container_attrmap = urwid.AttrMap(self._container,
                                                 "plugin.widget.page")
@@ -723,7 +721,7 @@
                 continue
             fill = width - used
             if fill == width:
-                #Fake out empty entries
+                # Fake out empty entries
                 row = [(1, 0, 1), (0, 1)]
                 last_offset = 1
             if fill < 0:
diff --git a/src/ovirt/node/utils/__init__.py b/src/ovirt/node/utils/__init__.py
index 8c49c31..176b01c 100644
--- a/src/ovirt/node/utils/__init__.py
+++ b/src/ovirt/node/utils/__init__.py
@@ -406,5 +406,5 @@
         except:
             pass
             # BAAAAD
-            #raise RuntimeError("Failed to parse line: %s" % line)
+            # raise RuntimeError("Failed to parse line: %s" % line)
     return cfg
diff --git a/src/ovirt/node/utils/expose.py b/src/ovirt/node/utils/expose.py
index 7e87081..3128a54 100644
--- a/src/ovirt/node/utils/expose.py
+++ b/src/ovirt/node/utils/expose.py
@@ -142,7 +142,7 @@
     def __getitem__(self, path):
         """Get the item with the path <path> or raise a KeyError
         """
-        if not path in self:
+        if path not in self:
             raise KeyError
         return self.__find(path)
 
diff --git a/src/ovirt/node/utils/fs.py b/src/ovirt/node/utils/fs.py
index 48d1bab..7a0cace 100644
--- a/src/ovirt/node/utils/fs.py
+++ b/src/ovirt/node/utils/fs.py
@@ -330,7 +330,7 @@
     def of(self, fn):
         """Returns the backup file for the given file
         """
-        #assert fn in self.backups, "No backup for '%s'" % fn
+        # assert fn in self.backups, "No backup for '%s'" % fn
         if fn in self.backups:
             return self.backups[fn]
         return None
diff --git a/src/ovirt/node/utils/network.py b/src/ovirt/node/utils/network.py
index d27831d..6b31258 100644
--- a/src/ovirt/node/utils/network.py
+++ b/src/ovirt/node/utils/network.py
@@ -791,7 +791,7 @@
                     continue
                 vdev, vid, hdev = [field.strip()
                                    for field in line.split("|")]
-                if not hdev in vlans:
+                if hdev not in vlans:
                     vlans[hdev] = []
                 vlans[hdev].append((vdev, vid))
         except IOError as e:
@@ -885,6 +885,6 @@
         if not self.is_bond(mifname):
             raise RuntimeError("Can no delete '%s', it is no bond master" %
                                mifname)
-        #process.call(["ip", "link", "set", "dev", mifname, "down"])
-        #process.call(["ip", "link", "delete", mifname, "type", "bond"])
+        # process.call(["ip", "link", "set", "dev", mifname, "down"])
+        # process.call(["ip", "link", "delete", mifname, "type", "bond"])
         fs.File(self.bonding_masters_filename).write("-%s" % mifname)
diff --git a/src/ovirt/node/utils/storage.py b/src/ovirt/node/utils/storage.py
index 5792af4..b4133fd 100644
--- a/src/ovirt/node/utils/storage.py
+++ b/src/ovirt/node/utils/storage.py
@@ -110,7 +110,7 @@
 
         from ovirtnode.ovirtfunctions import get_live_disk
         name = get_live_disk()
-        if not "/dev/mapper" in name:
+        if "/dev/mapper" not in name:
             # FIXME explain ...
             name = "/dev/%s" % name.rstrip('0123456789')
         self._cached_live_disk_name = name
diff --git a/src/ovirt/node/utils/system.py b/src/ovirt/node/utils/system.py
index 4786e5e..8f42e19 100644
--- a/src/ovirt/node/utils/system.py
+++ b/src/ovirt/node/utils/system.py
@@ -787,7 +787,7 @@
         def __getitem__(self, key):
             if re.match(r'^(.*?)=', key):
                 argument = re.match(r'^(.*?)=', key).groups()[0]
-                #flags = re.match(r'^.*?=(.*)', key).groups()[0]
+                # flags = re.match(r'^.*?=(.*)', key).groups()[0]
                 if argument in self.items:
                     return self.items[argument]
             elif key in self.items:
diff --git a/src/ovirt/node/utils/virt.py b/src/ovirt/node/utils/virt.py
index 8b5e40e..5a4b951 100644
--- a/src/ovirt/node/utils/virt.py
+++ b/src/ovirt/node/utils/virt.py
@@ -91,7 +91,7 @@
             num_domains = str(con.numOfDomains())
     except libvirt.libvirtError:
         pass
-        #warning("Error while working with libvirt: %s" % e.message)
+        # warning("Error while working with libvirt: %s" % e.message)
     return num_domains
 
 
diff --git a/src/ovirt/node/valid.py b/src/ovirt/node/valid.py
index 0441b93..50b24ee 100644
--- a/src/ovirt/node/valid.py
+++ b/src/ovirt/node/valid.py
@@ -305,7 +305,7 @@
     def validate(self, value):
         is_valid = super(FQDN, self).validate(value)
 
-        #Don't bother checking if it's not a valid FQDN. Doctest madness.
+        # Don't bother checking if it's not a valid FQDN. Doctest madness.
         if is_valid:
             FQDNLength()(value)
         return is_valid


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I910bddc6ac82e362863d60e22b5ffc5ad922f77a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>



More information about the node-patches mailing list