Remove error classes for distinguishing D-Bus properties

All we're interested in is a useful error message and that can be generated more easily at the site the exception is raised. Signed-off-by: mulhern <[email protected]>
stratis-storage · Nov 4, 2024 · 10afe87 · 10afe87
1 parent 354ceb8
commit 10afe87
Show file tree

Hide file tree

Showing 10 changed files with 39 additions and 100 deletions.
diff --git a/src/stratis_cli/_actions/_logical.py b/src/stratis_cli/_actions/_logical.py
@@ -21,10 +21,9 @@
 from .._constants import Id, IdType
 from .._errors import (
     StratisCliEngineError,
-    StratisCliFsMergeScheduledChangeError,
-    StratisCliFsSizeLimitChangeError,
     StratisCliIncoherenceError,
     StratisCliNoChangeError,
+    StratisCliNoPropertyChangeError,
     StratisCliPartialChangeError,
 )
 from .._stratisd_constants import StratisdErrors
@@ -302,8 +301,9 @@ def set_size_limit(namespace):  # pylint: disable=too-many-locals
         valid, maybe_size_limit = mofs.SizeLimit()
 
         if valid and new_limit.magnitude == int(str(maybe_size_limit)):
-            raise StratisCliFsSizeLimitChangeError(
-                new_limit if limit == "current" else user_input
+            raise StratisCliNoPropertyChangeError(
+                "Filesystem size limit is exactly "
+                f'{new_limit if limit == "current" else user_input}'
             )
 
         Filesystem.Properties.SizeLimit.Set(
@@ -334,7 +334,7 @@ def unset_size_limit(namespace):
         valid, _ = MOFilesystem(fs_info).SizeLimit()
 
         if not valid:
-            raise StratisCliFsSizeLimitChangeError(None)
+            raise StratisCliNoPropertyChangeError("Filesystem size limit is not set")
 
         Filesystem.Properties.SizeLimit.Set(get_object(fs_object_path), (False, ""))
 
@@ -364,7 +364,9 @@ def schedule_revert(namespace):
         merge_requested = MOFilesystem(fs_info).MergeScheduled()
 
         if bool(merge_requested):
-            raise StratisCliFsMergeScheduledChangeError(True)
+            raise StratisCliNoPropertyChangeError(
+                "Filesystem is already scheduled for a revert operation"
+            )
 
         Filesystem.Properties.MergeScheduled.Set(get_object(fs_object_path), True)
 
@@ -395,6 +397,8 @@ def cancel_revert(namespace):
         (merge_requested, (origin_set, _)) = (mofs.MergeScheduled(), mofs.Origin())
 
         if origin_set and not bool(merge_requested):
-            raise StratisCliFsMergeScheduledChangeError(False)
+            raise StratisCliNoPropertyChangeError(
+                "Filesystem is not currently scheduled for a revert operation"
+            )
 
         Filesystem.Properties.MergeScheduled.Set(get_object(fs_object_path), False)
diff --git a/src/stratis_cli/_actions/_pool.py b/src/stratis_cli/_actions/_pool.py
@@ -29,15 +29,13 @@
 from .._error_codes import PoolErrorCode
 from .._errors import (
     StratisCliEngineError,
-    StratisCliFsLimitChangeError,
-    StratisCliHasCacheChangeError,
     StratisCliIncoherenceError,
     StratisCliInUseOtherTierError,
     StratisCliInUseSameTierError,
     StratisCliNameConflictError,
     StratisCliNoChangeError,
     StratisCliNoDeviceSizeChangeError,
-    StratisCliOverprovisionChangeError,
+    StratisCliNoPropertyChangeError,
     StratisCliPartialChangeError,
     StratisCliResourceNotFoundError,
 )
@@ -332,7 +330,9 @@ def init_cache(namespace):  # pylint: disable=too-many-locals
         )
 
         if MOPool(pool_info).HasCache():
-            raise StratisCliHasCacheChangeError()
+            raise StratisCliNoPropertyChangeError(
+                "Pool already has an initialized cache"
+            )
 
         blockdevs = frozenset([os.path.abspath(p) for p in namespace.blockdevs])
 
@@ -707,7 +707,9 @@ def set_fs_limit(namespace):
         )
 
         if namespace.amount == MOPool(pool_info).FsLimit():
-            raise StratisCliFsLimitChangeError(namespace.amount)
+            raise StratisCliNoPropertyChangeError(
+                f"Pool filesystem limit is exactly {str(namespace.amount).lower()}"
+            )
 
         Pool.Properties.FsLimit.Set(get_object(pool_object_path), namespace.amount)
 
@@ -730,7 +732,9 @@ def set_overprovisioning_mode(namespace):
         )
 
         if decision == MOPool(pool_info).Overprovisioning():
-            raise StratisCliOverprovisionChangeError(decision)
+            raise StratisCliNoPropertyChangeError(
+                f"Pool's overprovision mode is already set to {str(decision).lower()}"
+            )
 
         Pool.Properties.Overprovisioning.Set(get_object(pool_object_path), decision)
 

diff --git a/src/stratis_cli/_errors.py b/src/stratis_cli/_errors.py
@@ -61,75 +61,6 @@ class StratisCliNoPropertyChangeError(StratisCliUserError):
     value.
     """
 
-    def __init__(self, value):
-        """
-        Initializer.
-
-        :param object value: property value that was not changed
-        """
-        # pylint: disable=super-init-not-called
-        self.value = value
-
-
-class StratisCliOverprovisionChangeError(StratisCliNoPropertyChangeError):
-    """
-    Raised when the user requests the same overprovision state that the pool
-    already has.
-    """
-
-    def __str__(self):
-        return f"Pool's overprovision mode is already set to {str(self.value).lower()}"
-
-
-class StratisCliFsLimitChangeError(StratisCliNoPropertyChangeError):
-    """
-    Raised when the user requests the same FsLimit value that the pool already
-    has.
-    """
-
-    def __str__(self):
-        return f"Pool's filesystem limit is exactly {str(self.value).lower()}"
-
-
-class StratisCliFsSizeLimitChangeError(StratisCliNoPropertyChangeError):
-    """
-    Raised when the user requests the same filesystems SizeLimit value that the
-    filesystem already has.
-    """
-
-    def __str__(self):
-        if self.value is None:
-            return "Filesystem's size limit is not set"
-        return f"Filesystem's size limit is exactly {self.value}"
-
-
-class StratisCliFsMergeScheduledChangeError(StratisCliNoPropertyChangeError):
-    """
-    Raised when the user requests the same filesystems MergeScheduled value
-    that the filesystem already has.
-    """
-
-    def __str__(self):
-        if self.value:
-            return "Filesystem is already scheduled for a revert operation."
-        return "Filesystem is not currently scheduled for a revert operation."
-
-
-class StratisCliHasCacheChangeError(StratisCliNoPropertyChangeError):
-    """
-    Raised when the user request cache initialization, but a cache is already
-    present. It is not currently possible to remove a cache.
-    """
-
-    def __init__(self):
-        """
-        Initialization.
-        """
-        super().__init__(True)
-
-    def __str__(self):
-        return "Pool already has an initialized cache."
-
 
 class StratisCliResourceNotFoundError(StratisCliUserError):
     """

diff --git a/tests/whitebox/integration/logical/test_cancel_revert.py b/tests/whitebox/integration/logical/test_cancel_revert.py
@@ -20,7 +20,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliFsMergeScheduledChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._misc import RUNNER, SimTestCase, device_name_list
 
@@ -68,7 +68,7 @@ def test_cancel_revert_when_unscheduled(self):
         Cancelling an unscheduled revert should fail.
         """
         command_line = self._MENU + [self._POOLNAME, self._SNAPNAME]
-        self.check_error(StratisCliFsMergeScheduledChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
     def test_cancel_revert_when_no_origin(self):
         """
@@ -91,4 +91,4 @@ def test_cancel_a_revert_twice(self):
         RUNNER(command_line)
         command_line = self._MENU + [self._POOLNAME, self._SNAPNAME]
         RUNNER(command_line)
-        self.check_error(StratisCliFsMergeScheduledChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
diff --git a/tests/whitebox/integration/logical/test_schedule_revert.py b/tests/whitebox/integration/logical/test_schedule_revert.py
@@ -20,7 +20,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliFsMergeScheduledChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list
 
@@ -72,7 +72,7 @@ def test_schedule_revert_twice(self):
             self._SNAPNAME,
         ]
         RUNNER(command_line)
-        self.check_error(StratisCliFsMergeScheduledChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
     def test_schedule_revert_once(self):
         """

diff --git a/tests/whitebox/integration/logical/test_set_size_limit.py b/tests/whitebox/integration/logical/test_set_size_limit.py
@@ -20,7 +20,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliFsSizeLimitChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list
 
@@ -54,15 +54,15 @@ def test_set_size_limit_twice(self):
         """
         command_line = self._MENU + [self._POOLNAME, self._FSNAME, "2TiB"]
         RUNNER(command_line)
-        self.check_error(StratisCliFsSizeLimitChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
     def test_set_size_limit_current_twice(self):
         """
         Use "current" to set the size limit to the same value twice.
         """
         command_line = self._MENU + [self._POOLNAME, self._FSNAME, "current"]
         RUNNER(command_line)
-        self.check_error(StratisCliFsSizeLimitChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
     def test_set_size_limit_exact(self):
         """

diff --git a/tests/whitebox/integration/logical/test_unset_size_limit.py b/tests/whitebox/integration/logical/test_unset_size_limit.py
@@ -17,7 +17,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliFsSizeLimitChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list
 
@@ -57,7 +57,7 @@ def test_unset_size_limit_twice(self):
         """
         command_line = self._MENU + [self._POOLNAME, self._FSNAME]
         RUNNER(command_line)
-        self.check_error(StratisCliFsSizeLimitChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
     def test_unset_size_limit_once(self):
         """

diff --git a/tests/whitebox/integration/pool/test_init_cache.py b/tests/whitebox/integration/pool/test_init_cache.py
@@ -20,7 +20,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliHasCacheChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._keyutils import RandomKeyTmpFile
 from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list
@@ -58,7 +58,7 @@ def test_init_cache(self):
         RUNNER(command_line)
 
         command_line = self._MENU + [self._POOLNAME] + _DEVICE_STRATEGY()
-        self.check_error(StratisCliHasCacheChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
 
 class InitCacheFail2TestCase(SimTestCase):
@@ -89,7 +89,7 @@ def test_init_cache(self):
         devices = _DEVICE_STRATEGY()
         command_line = self._MENU + [self._POOLNAME] + devices
         RUNNER(command_line)
-        self.check_error(StratisCliHasCacheChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
 
 class InitCacheFail3TestCase(SimTestCase):

diff --git a/tests/whitebox/integration/pool/test_set_fs_limit.py b/tests/whitebox/integration/pool/test_set_fs_limit.py
@@ -20,7 +20,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliFsLimitChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list
 
@@ -68,4 +68,4 @@ def test_set_fs_limit_same(self):
         RUNNER(command_line)
 
         command_line = self._MENU + [self._POOLNAME, fs_limit_value]
-        self.check_error(StratisCliFsLimitChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
diff --git a/tests/whitebox/integration/pool/test_set_overprovisioning.py b/tests/whitebox/integration/pool/test_set_overprovisioning.py
@@ -17,7 +17,7 @@
 
 # isort: LOCAL
 from stratis_cli import StratisCliErrorCodes
-from stratis_cli._errors import StratisCliOverprovisionChangeError
+from stratis_cli._errors import StratisCliNoPropertyChangeError
 
 from .._misc import RUNNER, TEST_RUNNER, SimTestCase, device_name_list
 
@@ -46,7 +46,7 @@ def test_set_overprovision_true_true(self):
         Test setting overprovision mode to true when true.
         """
         command_line = self._MENU + [self._POOLNAME] + ["yes"]
-        self.check_error(StratisCliOverprovisionChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)
 
     def test_set_overprovision_true_false(self):
         """
@@ -89,4 +89,4 @@ def test_set_overprovision_false_false(self):
         Test setting overprovision mode to false.
         """
         command_line = self._MENU + [self._POOLNAME] + ["no"]
-        self.check_error(StratisCliOverprovisionChangeError, command_line, _ERROR)
+        self.check_error(StratisCliNoPropertyChangeError, command_line, _ERROR)