Add more refined exception classes (#290)

Instead of wrapping everything under a single ProviderError, let's add
some nuance.

broker/broker.py

@@ -82,12 +82,13 @@ class Broker:
     """Main Broker class to be used as the primary interface for the Broker API."""
 
     # map exceptions for easier access when used as a library
-    BrokerError = exceptions.BrokerError
     AuthenticationError = exceptions.AuthenticationError
-    PermissionError = exceptions.PermissionError
-    ProviderError = exceptions.ProviderError
+    BrokerError = exceptions.BrokerError
     ConfigurationError = exceptions.ConfigurationError
     NotImplementedError = exceptions.NotImplementedError
+    PermissionError = exceptions.PermissionError
+    ProviderError = exceptions.ProviderError
+    UserError = exceptions.UserError
 
     def __init__(self, **kwargs):
         kwargs = helpers.resolve_file_args(kwargs)

broker/exceptions.py

@@ -72,3 +72,9 @@ class BeakerBindError(BrokerError):
     """Raised when a problem occurs at the Beaker bind level."""
 
     error_code = 12
+
+
+class UserError(BrokerError):
+    """Raised when a user causes an otherwise unclassified error."""
+
+    error_code = 13

broker/providers/__init__.py

@@ -155,16 +155,9 @@ def _validate_settings(self, instance_name=None):
     def _set_attributes(self, obj, attrs):
         obj.__dict__.update(attrs)
 
-    def _get_params(self, arg_list, kwargs):
-        return {k: v for k, v in kwargs.items() if k in arg_list}
-
+    @abstractmethod
     def construct_host(self, host_cls, provider_params, **kwargs):
         """Construct a host object from a host class and include relevent provider params."""
-        host_inst = host_cls(**provider_params, **kwargs)
-        host_attrs = self._get_params(self._construct_params)
-        host_attrs["release"] = self._host_release
-        self._set_attributes(host_inst, host_attrs)
-        return host_inst
 
     @abstractmethod
     def provider_help(self):

broker/providers/ansible_tower.py

@@ -17,14 +17,32 @@
 try:
     import awxkit
 except ImportError as err:
-    raise exceptions.ProviderError(
-        provider="AnsibleTower", message="Unable to import awxkit. Is it installed?"
-    ) from err
+    raise exceptions.UserError(message="Unable to import awxkit. Is it installed?") from err
 
 from broker import helpers
 from broker.providers import Provider
 
 
+class JobExecutionError(exceptions.ProviderError):
+    """Raised when a job execution fails."""
+
+    def __init__(self, message_data=None):
+        super().__init__(
+            provider="AnsibleTower",
+            message=json.dumps(message_data, indent=2),
+        )
+
+
+class ATInventoryError(exceptions.ProviderError):
+    """Raised when we can't find the right inventory."""
+
+    def __init__(self, message_data=None):
+        super().__init__(
+            provider="AnsibleTower",
+            message=json.dumps(message_data, indent=2),
+        )
+
+
 @cache
 def get_awxkit_and_uname(config=None, root=None, url=None, token=None, uname=None, pword=None):
     """Return an awxkit api object and resolved username."""
@@ -51,8 +69,7 @@ def get_awxkit_and_uname(config=None, root=None, url=None, token=None, uname=Non
             my_username = uname or versions.v2.get().me.get().results[0].username
         except (IndexError, AttributeError) as err:
             # lookup failed for whatever reason
-            raise exceptions.ProviderError(
-                provider="AnsibleTower",
+            raise exceptions.ConfigurationError(
                 message="Failed to lookup a username for the given token, please check credentials",
             ) from err
     else:  # dynaconf validators should have checked that either token or password was provided
@@ -203,8 +220,7 @@ def _translate_inventory(self, inventory):
             if inventory_info := self.v2.inventory.get(id=inventory):
                 return inventory_info.results[0].name
             else:
-                raise exceptions.ProviderError(
-                    provider="AnsibleTower",
+                raise ATInventoryError(
                     message=f"Unknown AnsibleTower inventory by id {inventory}",
                 )
         elif isinstance(inventory, str):
@@ -214,15 +230,13 @@ def _translate_inventory(self, inventory):
                     filtered = [inv for inv in inventory_info.results if inv.name == inventory]
                     if len(filtered) == 1:
                         return filtered[0].id
-                    raise exceptions.ProviderError(
-                        provider="AnsibleTower",
+                    raise ATInventoryError(
                         message=f"Ambigious AnsibleTower inventory name {inventory}",
                     )
                 elif inventory_info.count == 1:
                     return inventory_info.results.pop().id
                 else:
-                    raise exceptions.ProviderError(
-                        provider="AnsibleTower",
+                    raise ATInventoryError(
                         message=f"Unknown AnsibleTower inventory {inventory}",
                     )
         elif inv_id := getattr(inventory, "id", None):
@@ -231,8 +245,7 @@ def _translate_inventory(self, inventory):
             return inv_name
         else:
             caller_context = inspect.stack()[1][0].f_locals
-            raise exceptions.ProviderError(
-                provider="AnsibleTower",
+            raise ATInventoryError(
                 message=f"Ambiguous AnsibleTower inventory {inventory} passed from {caller_context}",
             )
 
@@ -573,20 +586,15 @@ def execute(self, **kwargs):  # noqa: PLR0912,PLR0915 - Possible TODO refactor
             subject = "job_template"
             get_path = self.v2.job_templates
         else:
-            raise exceptions.ProviderError(
-                provider="AnsibleTower", message="No workflow or job template specified"
-            )
+            raise exceptions.UserError(message="No workflow or job template specified")
         try:
             candidates = get_path.get(name=name).results
         except awxkit.exceptions.Unauthorized as err:
             raise exceptions.AuthenticationError(err.args[0]) from err
         if candidates:
             target = candidates.pop()
         else:
-            raise exceptions.ProviderError(
-                provider="AnsibleTower",
-                message=f"{subject.capitalize()} not found by name: {name}",
-            )
+            raise exceptions.UserError(message=f"{subject.capitalize()} not found by name: {name}")
         payload = {}
         if inventory := kwargs.pop("inventory", None):
             payload["inventory"] = inventory
@@ -636,9 +644,7 @@ def execute(self, **kwargs):  # noqa: PLR0912,PLR0915 - Possible TODO refactor
                 "URL": job_ui_url,
             }
             helpers.emit(message_data)
-            raise exceptions.ProviderError(
-                provider="AnsibleTower", message=message_data["Reason(s)"]
-            )
+            raise JobExecutionError(message_data=message_data)
         if strategy := kwargs.pop("artifacts", None):
             return self._merge_artifacts(job, strategy=strategy)
         return job

tests/providers/test_ansible_tower.py

@@ -102,7 +102,7 @@ def test_host_creation(tower_stub):
 
 
 def test_workflow_lookup_failure(tower_stub):
-    with pytest.raises(Broker.ProviderError) as err:
+    with pytest.raises(Broker.UserError) as err:
         tower_stub.execute(workflow="this-does-not-exist")
     assert "Workflow not found by name: this-does-not-exist" in err.value.message