[master 8/24] Remove action-oriented wrapper methods from DeviceTree.

vathpela installerbot-noreply at redhat.com
Fri Nov 6 18:31:03 UTC 2015


From: David Lehman <dlehman at redhat.com>

This replaces ActionList.append with ActionList.add. While perhaps
less intuitive to python people, 'add' and 'remove' are the most
obvious ways to manipulate a list.

DeviceTree methods removed:
  register_action->actions.add
  cancel_action->actions.remove
  find_actions->actions.find
  prune_actions->actions.prune
  sort_actions->actions.sort
  process_actions->actions.process
---
 blivet/actionlist.py     | 17 +++++++++++--
 blivet/blivet.py         | 26 ++++++++++----------
 blivet/devicetree.py     | 64 +++++++-----------------------------------------
 blivet/partitioning.py   |  2 +-
 tests/action_test.py     | 22 ++++++++---------
 tests/storagetestcase.py |  8 +++---
 6 files changed, 53 insertions(+), 86 deletions(-)

diff --git a/blivet/actionlist.py b/blivet/actionlist.py
index cf42028..875a451 100644
--- a/blivet/actionlist.py
+++ b/blivet/actionlist.py
@@ -36,18 +36,31 @@
 
 class ActionList(object):
 
-    def __init__(self):
+    def __init__(self, addfunc=None, removefunc=None):
+        self._add_func = addfunc
+        self._remove_func = removefunc
         self._actions = []
         self._completed_actions = []
 
     def __iter__(self):
         return iter(self._actions)
 
-    def append(self, action):
+    def add(self, action):
+        if self._add_func is not None:
+            self._add_func(action)
+
+        # apply the action before adding it in case apply raises an exception
+        action.apply()
         self._actions.append(action)
+        log.info("registered action: %s", action)
 
     def remove(self, action):
+        if self._remove_func:
+            self._remove_func(action)
+
+        action.cancel()
         self._actions.remove(action)
+        log.info("canceled action %s", action)
 
     def find(self, device=None, action_type=None, object_type=None,
              path=None, devid=None):
diff --git a/blivet/blivet.py b/blivet/blivet.py
index aa1a98e..53e95dd 100644
--- a/blivet/blivet.py
+++ b/blivet/blivet.py
@@ -168,7 +168,7 @@ def do_it(self, callbacks=None):
 
         """
 
-        self.devicetree.process_actions(callbacks=callbacks)
+        self.devicetree.actions.process(callbacks=callbacks)
         if not flags.installer_mode:
             return
 
@@ -683,7 +683,7 @@ def initialize_disk(self, disk):
 
         # remove existing formatting from the disk
         destroy_action = ActionDestroyFormat(disk)
-        self.devicetree.register_action(destroy_action)
+        self.devicetree.actions.add(destroy_action)
 
         label_type = _platform.best_disklabel_type(disk)
 
@@ -691,7 +691,7 @@ def initialize_disk(self, disk):
         new_label = get_format("disklabel", device=disk.path,
                                label_type=label_type)
         create_action = ActionCreateFormat(disk, fmt=new_label)
-        self.devicetree.register_action(create_action)
+        self.devicetree.actions.add(create_action)
 
     def remove_empty_extended_partitions(self):
         for disk in self.partitioned:
@@ -1078,9 +1078,9 @@ def create_device(self, device):
             :type device: :class:`~.devices.StorageDevice`
             :rtype: None
         """
-        self.devicetree.register_action(ActionCreateDevice(device))
+        self.devicetree.actions.add(ActionCreateDevice(device))
         if device.format.type and not device.format_immutable:
-            self.devicetree.register_action(ActionCreateFormat(device))
+            self.devicetree.actions.add(ActionCreateFormat(device))
 
     def destroy_device(self, device):
         """ Schedule destruction of a device.
@@ -1095,10 +1095,10 @@ def destroy_device(self, device):
         if device.format.exists and device.format.type and \
            not device.format_immutable:
             # schedule destruction of any formatting while we're at it
-            self.devicetree.register_action(ActionDestroyFormat(device))
+            self.devicetree.actions.add(ActionDestroyFormat(device))
 
         action = ActionDestroyDevice(device)
-        self.devicetree.register_action(action)
+        self.devicetree.actions.add(action)
 
     def format_device(self, device, fmt):
         """ Schedule formatting of a device.
@@ -1117,8 +1117,8 @@ def format_device(self, device, fmt):
         if device.protected:
             raise ValueError("cannot modify protected device")
 
-        self.devicetree.register_action(ActionDestroyFormat(device))
-        self.devicetree.register_action(ActionCreateFormat(device, fmt))
+        self.devicetree.actions.add(ActionDestroyFormat(device))
+        self.devicetree.actions.add(ActionCreateFormat(device, fmt))
 
     def reset_device(self, device):
         """ Cancel all scheduled actions and reset formatting.
@@ -1127,9 +1127,9 @@ def reset_device(self, device):
             :type device: :class:`~.devices.StorageDevice`
             :rtype: None
         """
-        actions = self.devicetree.find_actions(device=device)
+        actions = self.devicetree.actions.find(device=device)
         for action in reversed(actions):
-            self.devicetree.cancel_action(action)
+            self.devicetree.actions.remove(action)
 
         # make sure any random overridden attributes are reset
         device.format = copy.deepcopy(device.original_format)
@@ -1164,7 +1164,7 @@ def resize_device(self, device, new_size):
             classes.reverse()
 
         for action_class in classes:
-            self.devicetree.register_action(action_class(device, new_size))
+            self.devicetree.actions.add(action_class(device, new_size))
 
     def format_by_default(self, device):
         """Return whether the device should be reformatted by default."""
@@ -1776,7 +1776,7 @@ def update_ksdata(self):
             fresh_disks = [d.name for d in self.disks if d.partitioned and
                            not d.format.exists]
 
-            destroy_actions = self.devicetree.find_actions(action_type="destroy",
+            destroy_actions = self.devicetree.actions.find(action_type="destroy",
                                                            object_type="device")
 
             cleared_partitions = []
diff --git a/blivet/devicetree.py b/blivet/devicetree.py
index 7613c98..fe10aa5 100644
--- a/blivet/devicetree.py
+++ b/blivet/devicetree.py
@@ -91,7 +91,8 @@ def reset(self, conf=None, passphrase=None, luks_dict=None,
         """ Reset the instance to its initial state. """
         # internal data members
         self._devices = []
-        self._actions = ActionList()
+        self._actions = ActionList(addfunc=self._register_action,
+                                   removefunc=self._cancel_action)
 
         # a list of all device names we encounter
         self.names = []
@@ -251,9 +252,9 @@ def recursive_remove(self, device, actions=True):
                 if actions:
                     if leaf.format.exists and not leaf.protected and \
                        not leaf.format_immutable:
-                        self.register_action(ActionDestroyFormat(leaf))
+                        self.actions.add(ActionDestroyFormat(leaf))
 
-                    self.register_action(ActionDestroyDevice(leaf))
+                    self.actions.add(ActionDestroyDevice(leaf))
                 else:
                     if not leaf.format_immutable:
                         leaf.format = None
@@ -263,13 +264,13 @@ def recursive_remove(self, device, actions=True):
 
         if not device.format_immutable:
             if actions:
-                self.register_action(ActionDestroyFormat(device))
+                self.actions.add(ActionDestroyFormat(device))
             else:
                 device.format = None
 
         if not device.is_disk:
             if actions:
-                self.register_action(ActionDestroyDevice(device))
+                self.actions.add(ActionDestroyDevice(device))
             else:
                 self._remove_device(device)
 
@@ -280,7 +281,7 @@ def recursive_remove(self, device, actions=True):
     def actions(self):
         return self._actions
 
-    def register_action(self, action):
+    def _register_action(self, action):
         """ Register an action to be performed at a later time.
 
             :param action: the action
@@ -305,12 +306,7 @@ def register_action(self, action):
                action.device.format.mountpoint in self.filesystems:
                 raise DeviceTreeError("mountpoint already in use")
 
-        # apply the action before adding it in case apply raises an exception
-        action.apply()
-        log.info("registered action: %s", action)
-        self._actions.append(action)
-
-    def cancel_action(self, action):
+    def _cancel_action(self, action):
         """ Cancel a registered action.
 
             :param action: the action
@@ -329,48 +325,6 @@ def cancel_action(self, action):
             # add the device back into the tree
             self._add_device(action.device, new=False)
 
-        action.cancel()
-        self._actions.remove(action)
-        log.info("canceled action %s", action)
-
-    def find_actions(self, device=None, action_type=None, object_type=None,
-                     path=None, devid=None):
-        """ Find all actions that match all specified parameters.
-
-            A value of None for any of the keyword arguments indicates that any
-            value is acceptable for that field.
-
-            :keyword device: device to match
-            :type device: :class:`~.devices.StorageDevice` or None
-            :keyword action_type: action type to match (eg: "create", "destroy")
-            :type action_type: str or None
-            :keyword object_type: operand type to match (eg: "device" or "format")
-            :type object_type: str or None
-            :keyword path: device path to match
-            :type path: str or None
-            :keyword devid: device id to match
-            :type devid: int or None
-            :returns: a list of matching actions
-            :rtype: list of :class:`~.deviceaction.DeviceAction`
-
-        """
-        return self._actions.find(device=device,
-                                  action_type=action_type,
-                                  object_type=object_type,
-                                  path=path,
-                                  devid=devid)
-
-    def prune_actions(self):
-        return self._actions.prune()
-
-    def sort_actions(self):
-        return self._actions.sort()
-
-    def process_actions(self, callbacks=None, dry_run=False):
-        self.actions.process(devices=self.devices,
-                             dry_run=dry_run,
-                             callbacks=callbacks)
-
     #
     # Device detection
     #
@@ -1002,7 +956,7 @@ def hide(self, device):
             cancel = [a for a in self._actions
                       if set(a.device.disks).intersection(disks)]
             for action in reversed(cancel):
-                self.cancel_action(action)
+                self.actions.remove(action)
 
         for d in self.get_children(device):
             self.hide(d)
diff --git a/blivet/partitioning.py b/blivet/partitioning.py
index 1371203..bf79928 100644
--- a/blivet/partitioning.py
+++ b/blivet/partitioning.py
@@ -560,7 +560,7 @@ def do_partitioning(storage):
     #     a PartitionDevice for it.
     for partition in storage.partitions:
         if not partition.exists and partition.is_extended and \
-           not storage.devicetree.find_actions(device=partition, action_type="create"):
+           not storage.devicetree.actions.find(device=partition, action_type="create"):
             storage.devicetree._remove_device(partition, modparent=False, force=True)
 
     partitions = storage.partitions[:]
diff --git a/tests/action_test.py b/tests/action_test.py
index 03cd2b5..14c9488 100644
--- a/tests/action_test.py
+++ b/tests/action_test.py
@@ -267,7 +267,7 @@ def test_action_registration(self):
         self.assertEqual(vg.isleaf, False)
         a = ActionDestroyDevice(vg)
         with self.assertRaises(ValueError):
-            self.storage.devicetree.register_action(a)
+            self.storage.devicetree.actions.add(a)
 
         # registering any action other than create for a device that's not in
         # the devicetree should fail
@@ -281,7 +281,7 @@ def test_action_registration(self):
         create_sdc1_format = ActionCreateFormat(sdc1, sdc1_format)
         create_sdc1_format.apply()
         with self.assertRaises(blivet.errors.DeviceTreeError):
-            self.storage.devicetree.register_action(create_sdc1_format)
+            self.storage.devicetree.actions.add(create_sdc1_format)
 
         sdc1_format.exists = True
         sdc1_format._resizable = True
@@ -289,33 +289,33 @@ def test_action_registration(self):
                                                 sdc1.size - Size("10 GiB"))
         resize_sdc1_format.apply()
         with self.assertRaises(blivet.errors.DeviceTreeError):
-            self.storage.devicetree.register_action(resize_sdc1_format)
+            self.storage.devicetree.actions.add(resize_sdc1_format)
 
         resize_sdc1 = ActionResizeDevice(sdc1, sdc1.size - Size("10 GiB"))
         resize_sdc1.apply()
         with self.assertRaises(blivet.errors.DeviceTreeError):
-            self.storage.devicetree.register_action(resize_sdc1)
+            self.storage.devicetree.actions.add(resize_sdc1)
 
         resize_sdc1.cancel()
         resize_sdc1_format.cancel()
 
         destroy_sdc1_format = ActionDestroyFormat(sdc1)
         with self.assertRaises(blivet.errors.DeviceTreeError):
-            self.storage.devicetree.register_action(destroy_sdc1_format)
+            self.storage.devicetree.actions.add(destroy_sdc1_format)
 
         destroy_sdc1 = ActionDestroyDevice(sdc1)
         with self.assertRaises(blivet.errors.DeviceTreeError):
-            self.storage.devicetree.register_action(destroy_sdc1)
+            self.storage.devicetree.actions.add(destroy_sdc1)
 
         # registering a device destroy action should cause the device to be
         # removed from the devicetree
         lv_root = self.storage.devicetree.get_device_by_name("VolGroup-lv_root")
         self.assertNotEqual(lv_root, None)
         a = ActionDestroyDevice(lv_root)
-        self.storage.devicetree.register_action(a)
+        self.storage.devicetree.actions.add(a)
         lv_root = self.storage.devicetree.get_device_by_name("VolGroup-lv_root")
         self.assertEqual(lv_root, None)
-        self.storage.devicetree.cancel_action(a)
+        self.storage.devicetree.actions.remove(a)
 
         # registering a device create action should cause the device to be
         # added to the devicetree
@@ -327,7 +327,7 @@ def test_action_registration(self):
                                name="sdd1", size=Size("100 GiB"),
                                parents=[sdd])
         a = ActionCreateDevice(sdd1)
-        self.storage.devicetree.register_action(a)
+        self.storage.devicetree.actions.add(a)
         sdd1 = self.storage.devicetree.get_device_by_name("sdd1")
         self.assertNotEqual(sdd1, None)
 
@@ -1108,8 +1108,8 @@ def test_container_actions(self):
         self.assertEqual(create_new_lv.requires(add_sdc1), True)
         self.assertEqual(create_new_lv_format.requires(add_sdc1), False)
 
-        self.storage.devicetree.cancel_action(create_new_lv_format)
-        self.storage.devicetree.cancel_action(create_new_lv)
+        self.storage.devicetree.actions.remove(create_new_lv_format)
+        self.storage.devicetree.actions.remove(create_new_lv)
 
         remove_sdb1 = ActionRemoveMember(vg, sdb1)
         self.assertEqual(len(vg.parents), 3)
diff --git a/tests/storagetestcase.py b/tests/storagetestcase.py
index 4b58f59..3a66740 100644
--- a/tests/storagetestcase.py
+++ b/tests/storagetestcase.py
@@ -214,7 +214,7 @@ def schedule_create_device(self, device):
 
         self.assertEqual(devicetree.get_device_by_name(device.name), None)
         action = blivet.deviceaction.ActionCreateDevice(device)
-        devicetree.register_action(action)
+        devicetree.actions.add(action)
         self.assertEqual(devicetree.get_device_by_name(device.name), device)
         return action
 
@@ -231,7 +231,7 @@ def schedule_destroy_device(self, device):
 
         self.assertEqual(devicetree.get_device_by_name(device.name), device)
         action = blivet.deviceaction.ActionDestroyDevice(device)
-        devicetree.register_action(action)
+        devicetree.actions.add(action)
         self.assertEqual(devicetree.get_device_by_name(device.name), None)
         return action
 
@@ -250,7 +250,7 @@ def schedule_create_format(self, device, fmt):
         self.assertNotEqual(device.format, fmt)
         self.assertEqual(devicetree.get_device_by_name(device.name), device)
         action = blivet.deviceaction.ActionCreateFormat(device, fmt)
-        devicetree.register_action(action)
+        devicetree.actions.add(action)
         _device = devicetree.get_device_by_name(device.name)
         self.assertEqual(_device.format, fmt)
         return action
@@ -268,7 +268,7 @@ def schedule_destroy_format(self, device):
 
         self.assertEqual(devicetree.get_device_by_name(device.name), device)
         action = blivet.deviceaction.ActionDestroyFormat(device)
-        devicetree.register_action(action)
+        devicetree.actions.add(action)
         _device = devicetree.get_device_by_name(device.name)
         self.assertEqual(_device.format.type, None)
         return action


-- 
To view this commit on github, visit https://github.com/rhinstaller/blivet/commit/e297eab590bb68147196b0c4e391eea2e1bdf127


More information about the anaconda-patches mailing list