[node-patches] Change in ovirt-node[master]: ui: Generalize exception handling
fabiand at fedoraproject.org
fabiand at fedoraproject.org
Tue Feb 19 13:02:25 UTC 2013
Fabian Deutsch has uploaded a new change for review.
Change subject: ui: Generalize exception handling
......................................................................
ui: Generalize exception handling
Previously it was not possible to catch exceptions in callbacks and
display them in the TUI (without bringing the whole TUI down).
Now it's possible to catch exceptions in signal callbacks and display
them on the TUI screen.
Change-Id: I3cdec9ea5ea9f5777b25c44e9051234720fe55a7
Signed-off-by: Fabian Deutsch <fabiand at fedoraproject.org>
---
M scripts/tui/src/ovirt/node/app.py
M scripts/tui/src/ovirt/node/base.py
M scripts/tui/src/ovirt/node/plugins.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/utils/__init__.py
M scripts/tui/src/ovirt/node/utils/fs.py
M scripts/tui/src/ovirt/node/utils/storage.py
8 files changed, 118 insertions(+), 51 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-node refs/changes/98/12198/1
diff --git a/scripts/tui/src/ovirt/node/app.py b/scripts/tui/src/ovirt/node/app.py
index 982dcee..352333e 100644
--- a/scripts/tui/src/ovirt/node/app.py
+++ b/scripts/tui/src/ovirt/node/app.py
@@ -201,19 +201,38 @@
else:
window.close_topmost_dialog()
+ def call_on_ui_save(d):
+ self.current_plugin()._on_ui_save()
+
+ def call_on_ui_reset(d):
+ self.current_plugin()._on_ui_reset()
+
+ def call_on_ui_change(d):
+ self.current_plugin()._on_ui_change(d)
+
+ def call_on_ui_reload(d):
+ self.switch_to_plugin(self.current_plugin(), False)
+
+ def call_quit(d):
+ self.quit()
+
+ def display_exception_as_notice(e):
+ self.logger.debug(traceback.format_exc())
+ children = [ui.Label("dialog.notice.exception", "%s" % e)]
+ notice = ui.Dialog("dialog.notice", "An exception occurred",
+ children)
+ notice.buttons = [ui.CloseButton("dialog.notice.action.close",
+ "Close")]
+ self.show(notice)
+
# All known handlers
- handlers = {ui.SaveAction:
- lambda d: self.current_plugin()._on_ui_save(),
+ handlers = {ui.SaveAction: call_on_ui_save,
ui.CloseAction: cond_close_dialog,
- ui.ResetAction:
- lambda d: self.current_plugin()._on_ui_reset(),
- ui.ChangeAction:
- lambda d: self.current_plugin()._on_ui_change(d),
- ui.ReloadAction:
- lambda d: self.switch_to_plugin(self.current_plugin(),
- False),
- ui.QuitAction:
- lambda d: self.quit()
+ ui.ResetAction: call_on_ui_reset,
+ ui.ChangeAction: call_on_ui_change,
+ ui.ReloadAction: call_on_ui_reload,
+ ui.QuitAction: call_quit,
+ ui.DisplayExceptionNotice: display_exception_as_notice
}
for element in elements:
@@ -228,8 +247,9 @@
if type(element) is ui.SaveButton:
# 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)
+ def toggle_savebutton_disabled(t, v, e=element):
+ e.enabled(v)
+ plugin.on_valid.connect(toggle_savebutton_disabled)
def populate_with_values(self, ui_container):
"""Take values from model and inject them into the appropriate UI
diff --git a/scripts/tui/src/ovirt/node/base.py b/scripts/tui/src/ovirt/node/base.py
index 5130dbb..5d6d12f 100644
--- a/scripts/tui/src/ovirt/node/base.py
+++ b/scripts/tui/src/ovirt/node/base.py
@@ -65,11 +65,12 @@
def emit(self, userdata=None):
"""Emit a signal
"""
- target = self.target
+ self.logger.debug("Running %s callbacks: %s" % (self,
+ self.callbacks))
for idx, cb in enumerate(self.callbacks):
- self.logger.debug("(%d/%d) Emitting from %s: %s" %
+ self.logger.debug("(%d/%d) %s emits %s" %
(idx + 1, len(self.callbacks), self, cb))
- if cb(target, userdata) is False:
+ if cb(self.target, userdata) is False:
self.logger.debug("Breaking callback sequence")
return self
diff --git a/scripts/tui/src/ovirt/node/plugins.py b/scripts/tui/src/ovirt/node/plugins.py
index 5edb149..3ed9da4 100644
--- a/scripts/tui/src/ovirt/node/plugins.py
+++ b/scripts/tui/src/ovirt/node/plugins.py
@@ -282,7 +282,7 @@
self.on_change(change)
except exceptions.InvalidData as e:
self.on_valid(False)
- raise e
+ raise
self.__changes.update(change)
self.logger.debug("Sum of all UI changes up to now: %s" %
self.__changes)
diff --git a/scripts/tui/src/ovirt/node/ui/__init__.py b/scripts/tui/src/ovirt/node/ui/__init__.py
index 5641dc5..29e3cbf 100644
--- a/scripts/tui/src/ovirt/node/ui/__init__.py
+++ b/scripts/tui/src/ovirt/node/ui/__init__.py
@@ -34,9 +34,13 @@
Args:
path: The model path this item is mapped to
on_value_change: Emitted by the value() method if the value changes
+ on_exception: Signal to be called when an exception is raosed during a
+ callback
"""
path = None
on_value_change = None
+
+ on_exception = None
def __init__(self, path=None):
"""Registers all widget signals.
@@ -45,6 +49,7 @@
super(Element, self).__init__()
self.path = path
self.on_value_change = self.new_signal()
+ self.on_exception = self.new_signal()
self.logger.debug("Initializing %s" % self)
def value(self, value=None):
@@ -141,6 +146,11 @@
elements += element.elements()
return elements
+ def value(self, dummy):
+ """A container doesn't have a single value, therefor None
+ """
+ return NotImplementedError
+
def __getitem__(self, path):
return {c.path: c for c in self.children}[path]
@@ -148,12 +158,9 @@
class Action(base.Base):
callback = None
- on_callback_exception = None
-
def __init__(self, callback=None):
super(Action, self).__init__()
self.callback = callback
- self.on_callback_exception = self.new_signal()
def __call__(self, target, userdata=None):
r = None
@@ -162,15 +169,9 @@
self.callback,
userdata))
- try:
- r = self.callback(userdata)
- self.logger.debug("Action %s called and returned: %s" % (self,
- r))
- except Exception as e:
- if self.on_callback_exception.callbacks:
- self.on_callback_exception(e)
- else:
- raise e
+ r = self.callback(userdata)
+ self.logger.debug("Action %s called and returned: %s" % (self,
+ r))
else:
self.logger.warning("No callback for %s" % self)
return r
@@ -218,6 +219,12 @@
class QuitAction(Action):
"""Action to quit the application
+ """
+ pass
+
+
+class DisplayExceptionNotice(Action):
+ """Display a given Exception as a Notice
"""
pass
@@ -467,7 +474,7 @@
self.selection(selected_item or items[0][0])
self.on_activate.connect(ChangeAction())
self.on_activate.connect(SaveAction())
-
+ self.on_exception.connect(DisplayExceptionNotice())
def selection(self, selected=None):
"""Get/Select the given item (key) or multiple items if multi
diff --git a/scripts/tui/src/ovirt/node/ui/urwid_builder.py b/scripts/tui/src/ovirt/node/ui/urwid_builder.py
index 3f7de07..36657d9 100644
--- a/scripts/tui/src/ovirt/node/ui/urwid_builder.py
+++ b/scripts/tui/src/ovirt/node/ui/urwid_builder.py
@@ -18,15 +18,16 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
# MA 02110-1301, USA. A copy of the GNU General Public License is
# also available at http://www.gnu.org/copyleft/gpl.html.
+from ovirt.node import ui, exceptions, base
+from ovirt.node.exceptions import InvalidData
+from ovirt.node.ui import widgets as uw
+import urwid
"""
A visitor to build the urwid TUI from the abstract UI definitions.
Is based on the visitor pattern
"""
-from ovirt.node import ui, exceptions, base
-from ovirt.node.ui import widgets as uw
-import urwid
class UrwidUIBuilder(ui.AbstractUIBuilder):
@@ -71,6 +72,7 @@
self.logger.debug("Element changed, updating label " +
"'%s': %s" % (w, v))
widget.text(v)
+ self.application.ui.force_redraw()
ui_label.on_value_change.connect(on_item_text_change_cb)
return widget
@@ -87,7 +89,13 @@
def on_widget_click_cb(widget, data=None):
change = {ui_button.path: True}
self.logger.debug("Button click: %s" % change)
- ui_button.on_activate(change)
+ try:
+ ui_button.on_activate(change)
+ except Exception as e:
+ if ui_button.on_exception.callbacks:
+ ui_button.on_exception(e)
+ else:
+ raise
urwid.connect_signal(widget, "click", on_widget_click_cb)
@@ -161,14 +169,17 @@
widget.notice = ""
widget.valid(True)
- except exceptions.Concern as e:
- self.logger.error("Concern when updating: %s" % e)
-
except exceptions.InvalidData as e:
self.logger.error("Invalid data when updating: %s" % e)
if widget._selectable:
widget.notice = e.message
widget.valid(False)
+
+ except Exception as e:
+ if ui_entry.on_exception.callbacks:
+ ui_entry.on_exception(e)
+ else:
+ raise
#tui.force_redraw()
urwid.connect_signal(widget, 'change', on_widget_value_change)
@@ -188,7 +199,13 @@
def on_widget_change_cb(widget, data):
ui_options.option(data)
self.logger.debug("Options changed, calling callback: %s" % data)
- ui_options.on_change({ui_options.path: data})
+ try:
+ ui_options.on_change({ui_options.path: data})
+ except Exception as e:
+ if ui_options.on_exception.callbacks:
+ ui_options.on_exception(e)
+ else:
+ raise
urwid.connect_signal(widget, "change", on_widget_change_cb)
@@ -206,7 +223,13 @@
def on_widget_change_cb(widget, data=None):
ui_checkbox.state(data)
self.logger.debug("Checkbox changed, calling callback: %s" % data)
- ui_checkbox.on_change({ui_checkbox.path: data})
+ try:
+ ui_checkbox.on_change({ui_checkbox.path: data})
+ except Exception as e:
+ if ui_checkbox.on_exception.callbacks:
+ ui_checkbox.on_exception(e)
+ else:
+ raise
urwid.connect_signal(widget, "change", on_widget_change_cb)
return widget
@@ -249,7 +272,14 @@
ui_table.selection(widget.selection())
else:
ui_table.selection(w._key)
- ui_table.on_change({ui_table.path: w._key})
+
+ try:
+ ui_table.on_change({ui_table.path: w._key})
+ except Exception as e:
+ if ui_table.on_exception.callbacks:
+ ui_table.on_exception(e)
+ else:
+ raise
urwid.connect_signal(widget, "changed", on_change_cb)
@@ -265,10 +295,15 @@
c._key = key
def on_activate_cb(w, data):
- ui_table.selection(w._table.selection())
- ui_table.on_change({ui_table.path: w._key})
- ui_table.on_activate({ui_table.path: w._key})
-
+ try:
+ ui_table.selection(w._table.selection())
+ ui_table.on_change({ui_table.path: w._key})
+ ui_table.on_activate({ui_table.path: w._key})
+ except Exception as e:
+ if ui_table.on_exception.callbacks:
+ ui_table.on_exception(e)
+ else:
+ raise
urwid.connect_signal(c, "activate", on_activate_cb)
return c
diff --git a/scripts/tui/src/ovirt/node/utils/__init__.py b/scripts/tui/src/ovirt/node/utils/__init__.py
index cb33ac7..d38db8a 100644
--- a/scripts/tui/src/ovirt/node/utils/__init__.py
+++ b/scripts/tui/src/ovirt/node/utils/__init__.py
@@ -265,7 +265,7 @@
self.logger.warning("'%s' on transaction '%s': %s - %s" %
(type(e), self, e, e.message))
self.logger.debug(str(traceback.format_exc()))
- raise e
+ raise
self.logger.debug("Finished transaction %s successfully" % self)
class Element(base.Base):
diff --git a/scripts/tui/src/ovirt/node/utils/fs.py b/scripts/tui/src/ovirt/node/utils/fs.py
index 9114d0d..6cde158 100644
--- a/scripts/tui/src/ovirt/node/utils/fs.py
+++ b/scripts/tui/src/ovirt/node/utils/fs.py
@@ -66,7 +66,7 @@
os.rename(*fns)
except Exception as e:
backup.remove()
- raise e
+ raise
def truncate(filename):
diff --git a/scripts/tui/src/ovirt/node/utils/storage.py b/scripts/tui/src/ovirt/node/utils/storage.py
index 749cbb6..c8ea769 100644
--- a/scripts/tui/src/ovirt/node/utils/storage.py
+++ b/scripts/tui/src/ovirt/node/utils/storage.py
@@ -49,6 +49,8 @@
class Devices(base.Base):
"""A class to retrieve available storage devices
"""
+ _fake_devices = None
+
def __init__(self, fake=False):
super(Devices, self).__init__()
if fake:
@@ -94,18 +96,20 @@
"%s it's the live media" % dev)
continue
infos = disk_dict[dev].split(",", 5)
- device = Device(*infos)
+ device = Device(dev, *infos)
device.name = os.path.basename(device.name).replace(" ", "")
device.name = translate_multipath_device(device.name)
if device.name in devices:
self.logger.debug("Device with same name already " +
"exists: %s" % device.name)
- devices[device.name] = device
+ devices[device.path] = device
+ return devices
class Device(base.Base):
"""Wrapps the information about a udev storage device
"""
+ path = None
bus = None
name = None
size = None
@@ -113,10 +117,10 @@
serial = None
model = None
- def __init__(self, bus, name, size, desc, serial, model):
+ def __init__(self, path, bus, name, size, desc, serial, model):
super(Device, self).__init__()
- vals = [bus, name, size, desc, serial, model]
- props = ["bus", "name", "size", "desc", "serial", "model"]
+ vals = [path, bus, name, size, desc, serial, model]
+ props = ["path", "bus", "name", "size", "desc", "serial", "model"]
for prop, val in zip(props, vals):
self.__dict__[prop] = val
--
To view, visit http://gerrit.ovirt.org/12198
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3cdec9ea5ea9f5777b25c44e9051234720fe55a7
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