From a425f8bc6e582b6d8d50e77361a5ddaee98ec10c Mon Sep 17 00:00:00 2001
From: Adam Dyess <adam.dyess@canonical.com>
Date: Thu, 30 Jan 2025 15:10:31 -0600
Subject: [PATCH] Pythonic error handling

---
 src/manifests_cephfs.py             | 26 +++++++++++++-------------
 tests/unit/test_manifests_cephfs.py |  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/manifests_cephfs.py b/src/manifests_cephfs.py
index 2f732f1..581500f 100644
--- a/src/manifests_cephfs.py
+++ b/src/manifests_cephfs.py
@@ -4,7 +4,7 @@
 
 import logging
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
+from typing import TYPE_CHECKING, Any, Dict, List, Optional
 
 from lightkube.codecs import AnyResource
 from lightkube.models.core_v1 import Toleration
@@ -123,7 +123,7 @@ def create(self, param: CephStorageClassParameters) -> AnyResource:
             )
         )
 
-    def parameter_list(self) -> Tuple[List[CephStorageClassParameters], Optional[str]]:
+    def parameter_list(self) -> List[CephStorageClassParameters]:
         """Accumulate names and settings of the storage classes."""
         enabled = self.manifests.config.get("enabled")
         fsid = self.manifests.config.get("fsid")
@@ -132,19 +132,19 @@ def parameter_list(self) -> Tuple[List[CephStorageClassParameters], Optional[str
 
         if not enabled:
             log.info("Ignore CephFS Storage Class")
-            return [], None
+            return []
 
         if not fsid:
             log.error("CephFS is missing a filesystem: 'fsid'")
-            return [], "missing fsid"
+            raise ValueError("missing fsid")
 
         if not fs_data:
             log.error("CephFS is missing a filesystem listing: '%s'", self.FILESYSTEM_LISTING)
-            return [], "missing filesystem listing"
+            raise ValueError("missing filesystem listing")
 
         if not formatter:
             log.error("CephFS is missing '%s'", self.STORAGE_NAME_FORMATTER)
-            return [], f"empty {self.STORAGE_NAME_FORMATTER}"
+            raise ValueError(f"empty {self.STORAGE_NAME_FORMATTER}")
 
         sc_names: List[CephStorageClassParameters] = []
         for fs in fs_data:
@@ -169,13 +169,12 @@ def parameter_list(self) -> Tuple[List[CephStorageClassParameters], Optional[str
                 fs_data,
                 sc_names,
             )
-            return [], f"{self.STORAGE_NAME_FORMATTER} does not generate unique names"
-        return sc_names, None
+            raise ValueError(f"{self.STORAGE_NAME_FORMATTER} does not generate unique names")
+        return sc_names
 
     def __call__(self) -> List[AnyResource]:
         """Craft the storage class object."""
-        classes, _ = self.parameter_list()
-        return [self.create(class_param) for class_param in classes]
+        return [self.create(class_param) for class_param in self.parameter_list()]
 
 
 class ProvisionerAdjustments(Patch):
@@ -295,7 +294,8 @@ def evaluate(self) -> Optional[str]:
                 return f"CephFS manifests require the definition of '{prop}'"
 
         sc_manipulator = next(m for m in self.manipulations if isinstance(m, CephStorageClass))
-        _, err = sc_manipulator.parameter_list()
-        if err:
-            return f"CephFS manifests failed to create storage classes: {err=}"
+        try:
+            sc_manipulator.parameter_list()
+        except ValueError as err:
+            return f"CephFS manifests failed to create storage classes: {err}"
         return None
diff --git a/tests/unit/test_manifests_cephfs.py b/tests/unit/test_manifests_cephfs.py
index e6ac95b..4ceb264 100644
--- a/tests/unit/test_manifests_cephfs.py
+++ b/tests/unit/test_manifests_cephfs.py
@@ -154,7 +154,7 @@ def test_manifest_evaluation(caplog):
     assert manifests.evaluate() == "CephFS manifests require the definition of 'user'"
 
     charm.config["user"] = "cephx"
-    err_formatter = "CephFS manifests failed to create storage classes: err='{}'"
+    err_formatter = "CephFS manifests failed to create storage classes: {}"
     assert manifests.evaluate() == err_formatter.format("missing fsid")
 
     charm.config["fsid"] = "cluster"