[node-patches] Change in ovirt-node[master]: Several logging enhancements for better debugging

fabiand at fedoraproject.org fabiand at fedoraproject.org
Tue Dec 11 20:09:35 UTC 2012


Fabian Deutsch has uploaded a new change for review.

Change subject: Several logging enhancements for better debugging
......................................................................

Several logging enhancements for better debugging

Change-Id: I41739cec5294e0720845baf7df0b43a2a8190284
Signed-off-by: Fabian Deutsch <fabiand at fedoraproject.org>
---
M scripts/tui/src/ovirt/node/plugins/__init__.py
M scripts/tui/src/ovirt/node/plugins/network_page.py
M scripts/tui/src/ovirt/node/ui/__init__.py
M scripts/tui/src/ovirt/node/ui/builder.py
M scripts/tui/src/ovirt/node/utils/network.py
5 files changed, 72 insertions(+), 44 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/99/9899/1

diff --git a/scripts/tui/src/ovirt/node/plugins/__init__.py b/scripts/tui/src/ovirt/node/plugins/__init__.py
index 453a680..4f2a7ab 100644
--- a/scripts/tui/src/ovirt/node/plugins/__init__.py
+++ b/scripts/tui/src/ovirt/node/plugins/__init__.py
@@ -67,10 +67,8 @@
     Errors are propagated back by using Errors/Exceptions.
     """
 
-    _changes = None
-
     def __init__(self):
-        self._changes = {}
+        self.__changes = {}
 
     def name(self):
         """Returns the name of the plugin.
@@ -127,6 +125,15 @@
                     raise ovirt.node.exceptions.InvalidData(msg)
         return True
 
+    def ui_name(self):
+        """Returns the UI friendly name for this plugin.
+        Is e.g. used for the menu entry.
+
+        Returns:
+            Title of the plugin as a string
+        """
+        return self.name()
+
     def has_ui(self):
         """Determins if a page for this should be displayed in the UI
 
@@ -149,6 +156,7 @@
     def on_change(self, changes):
         """Applies the changes to the plugins model, will do all required logic
         return True if succeeds, otherwie false or throw an error
+        This is expected to do semanitcal checks on the model.
 
         Args:
             changes (dict): changes which shall be checked against the model
@@ -177,8 +185,11 @@
         except NotImplementedError:
             LOGGER.debug("Plugin has no model")
         except ovirt.node.exceptions.InvalidData:
-            LOGGER.warning("Plugin has invalid model")
+            LOGGER.warning("Plugins model does not pass sematic check: %s" % \
+                           model)
             is_valid = False
+        finally:
+            self.__changes = {}
         return is_valid
 
     def on_merge(self, effective_changes):
@@ -188,24 +199,23 @@
         Args:
             changes (dict): changes which shall be applied to the model
         Returns:
-            True on success, or False otherwie
+            True on success, or False otherwise
         Raises:
             Errors
         """
         raise NotImplementedError()
 
-    def ui_name(self):
-        return self.name()
-
     def _on_ui_change(self, change):
         """Called when some widget was changed
         change is expected to be a dict.
         """
-        assert type(change) is dict
-        LOGGER.debug("Model change: " + str(change))
+        if type(change) is not dict:
+            LOGGER.warning("Change is not a dict: %s" % change)
+
+        LOGGER.debug("Passing UI change to callback on_change: %s" % change)
         self.on_change(change)
-        self._changes.update(change)
-        LOGGER.debug(self._changes)
+        self.__changes.update(change)
+        LOGGER.debug("Sum of all UI changes up to now: %s" % self.__changes)
         return True
 
     def _on_ui_save(self):
@@ -213,15 +223,40 @@
         Calls merge_changes, but only with values that really changed
         """
         LOGGER.debug("Request to apply model changes")
-        real_changes = {}
-        if self._changes:
+        effective_changes = self.pending_changes() or {}
+        successfull_merge = self.on_merge(effective_changes)
+        if successfull_merge:
+            self.__changes = {}
+        return successfull_merge
+
+    def pending_changes(self, only_effective_changes=True):
+        """Return all changes which happened since the last on_merge call
+
+        Args:
+            only_effective_changes: Boolean if all or only the effective
+                changes are returned.
+        Returns:
+            dict of changes
+        """
+        return self.__effective_changes() if only_effective_changes \
+                                          else self.__changes
+
+    def __effective_changes(self):
+        """Calculates the effective changes, so changes which change the
+        value of a path.
+
+        Returns:
+            dict of effective changes
+        """
+        effective_changes = {}
+        if self.__changes:
             model = self.model()
-            for key, value in self._changes.items():
+            for key, value in self.__changes.items():
                 if key in model and value == model[key]:
                     LOGGER.debug(("Skipping pseudo-change of '%s', value " + \
-                                  "did not change") % key)
+                                  "(%s) did not change") % (key, value))
                 else:
-                    real_changes[key] = value
+                    effective_changes[key] = value
         else:
-            LOGGER.debug("No changes detected")
-        return self.on_merge(real_changes)
+            LOGGER.debug("No effective changes detected.")
+        return effective_changes if len(effective_changes) > 0 else None
diff --git a/scripts/tui/src/ovirt/node/plugins/network_page.py b/scripts/tui/src/ovirt/node/plugins/network_page.py
index aee4fa5..8bdf852 100644
--- a/scripts/tui/src/ovirt/node/plugins/network_page.py
+++ b/scripts/tui/src/ovirt/node/plugins/network_page.py
@@ -132,7 +132,7 @@
         pass
 
     def on_merge(self, effective_changes):
-        effective_model = self._model.update(changes)
+        effective_model = self._model.update(effective_changes)
 
         if "dns[0]" in effective_changes or \
            "dns[1]" in effective_changes:
diff --git a/scripts/tui/src/ovirt/node/ui/__init__.py b/scripts/tui/src/ovirt/node/ui/__init__.py
index 618a3e8..903af12 100644
--- a/scripts/tui/src/ovirt/node/ui/__init__.py
+++ b/scripts/tui/src/ovirt/node/ui/__init__.py
@@ -126,14 +126,10 @@
         self.children = children
         super(ContainerElement, self).__init__()
 
-    @property
-    @deprecated
-    def widgets(self):
+    def children(self, v=None):
+        if v:
+            self.children = v
         return self.children
-
-    @widgets.setter
-    def set_widgets(self, v):
-        self.children = v
 
 
 class Page(ContainerElement):
diff --git a/scripts/tui/src/ovirt/node/ui/builder.py b/scripts/tui/src/ovirt/node/ui/builder.py
index 7b4161c..971ce0c 100644
--- a/scripts/tui/src/ovirt/node/ui/builder.py
+++ b/scripts/tui/src/ovirt/node/ui/builder.py
@@ -60,7 +60,7 @@
     save = build_button("", ovirt.node.ui.SaveButton(), tui, plugin)
     plugin._save_button = save
 
-    for path, item in container.widgets:
+    for path, item in container.children:
         widget = widget_for_item(tui, plugin, path, item)
         widgets.append(("flow", widget))
 
@@ -128,14 +128,13 @@
     widget.enable(item.enabled)
 
     def on_item_enabled_change_cb(w, v):
-        LOGGER.debug("Model changed, updating widget '%s': %s" % (w,
-                                                                  v))
+        LOGGER.debug("Element changed, updating entry '%s': %s" % (w, v))
         if widget.selectable() != v:
             widget.enable(v)
     item.connect_signal("enabled", on_item_enabled_change_cb)
 
     def on_widget_value_change(widget, new_value):
-        LOGGER.debug("Widget changed, updating model '%s'" % path)
+        LOGGER.debug("Entry changed, calling callback: '%s'" % path)
 
         try:
             change = {path: new_value}
@@ -143,7 +142,6 @@
             plugin._on_ui_change(change)
             widget.notice = ""
             widget.valid(True)
-            LOGGER.debug(plugin.__dict__)
             plugin._save_button.enable(True)
 
         except ovirt.node.exceptions.Concern as e:
@@ -174,8 +172,7 @@
         widget = ovirt.node.ui.widgets.Label(item.text())
 
     def on_item_text_change_cb(w, v):
-        LOGGER.debug("Model changed, updating widget '%s': %s" % (w,
-                                                                  v))
+        LOGGER.debug("Element changed, updating label '%s': %s" % (w, v))
         widget.text(v)
         # Redraw the screen if widget text is updated "outside" of the
         # mainloop
@@ -192,7 +189,7 @@
         LOGGER.debug("Button click: %s" % widget)
         if type(item) is ovirt.node.ui.SaveButton:
             r = plugin._on_ui_save()
-            LOGGER.debug("Got save: %s" % r)
+            LOGGER.debug("SaveButton clicked: %s" % r)
 
             if type(r) in [ovirt.node.ui.Page]:
                 w = build_page(tui, plugin, r)
@@ -225,8 +222,8 @@
                                            plugin.model()[path])
 
     def on_widget_change_cb(widget, data):
-        LOGGER.debug(data)
         item.option(data)
+        LOGGER.debug("Options changed, calling callback: %s" % data)
         plugin._on_ui_change({path: data})
     urwid.connect_signal(widget, "change", on_widget_change_cb)
 
@@ -246,8 +243,7 @@
     widget = ovirt.node.ui.widgets.ProgressBarWidget(item.current(), item.done)
 
     def on_item_current_change_cb(w, v):
-        LOGGER.debug("Model changed, updating widget '%s': %s" % (w,
-                                                                  v))
+        LOGGER.debug("Model changed, updating progressbar '%s': %s" % (w, v))
         widget.set_completion(v)
         tui.draw_screen()
     item.connect_signal("current", on_item_current_change_cb)
@@ -265,6 +261,7 @@
 
     return widget
 
+
 def _build_tableitem(path, plugin, key, label):
     c = ovirt.node.ui.widgets.TableEntryWidget(label)
     urwid.connect_signal(c, "click",
diff --git a/scripts/tui/src/ovirt/node/utils/network.py b/scripts/tui/src/ovirt/node/utils/network.py
index 42d9a45..33b6dd8 100644
--- a/scripts/tui/src/ovirt/node/utils/network.py
+++ b/scripts/tui/src/ovirt/node/utils/network.py
@@ -72,7 +72,8 @@
 #                "udev informations are incomplete (udevadm re-trigger?)"
 
         info = {"name": d.get_property("INTERFACE"),
-                "vendor": d.get_property("ID_VENDOR_FROM_DATABASE") or "unkown",
+                "vendor": d.get_property("ID_VENDOR_FROM_DATABASE") \
+                          or "unkown",
                 "devtype": d.get_property("DEVTYPE") or "unknown",
                 "devpath": d.get_property("DEVPATH")
                }
@@ -95,10 +96,10 @@
             try:
                 driver = os.path.basename(os.readlink(driver_symlink))
             except Exception as e:
-                LOGGER.warning("Exception while reading driver " +
-                               "of '%s' from '%s'" % (name, driver_symlink))
+                LOGGER.warning(("Exception %s while reading driver " +
+                                "of '%s' from '%s'") % (e, name,
+                                                        driver_symlink))
         infos[name]["driver"] = driver
-
 
         hwaddr = "unkown"
         with open("/sys/class/net/%s/address" % name) as macfile:
@@ -131,7 +132,6 @@
             info["parent"] = ".".join(parts[:-1])
 
             info["type"] = "vlan"
-
 
     return infos
 
@@ -226,7 +226,7 @@
         servers.append(aug.get(path))
 
     if new_servers:
-        itempath = lambda idx: "%s[%d]" % (augpath, idx+1)
+        itempath = lambda idx: "%s[%d]" % (augpath, idx + 1)
         for idx, server in enumerate(new_servers):
             aug.set(itempath(idx), server)
         if len(servers) > len(new_servers):


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I41739cec5294e0720845baf7df0b43a2a8190284
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