Minor changes to logging and exceptions

This change addresses some minor issues we've seen with a new exception.
Additionally, it simplifies the AnsibleTower instance repr in logging.

broker/providers/ansible_tower.py

@@ -36,10 +36,10 @@ def __init__(self, message_data=None):
 class ATInventoryError(exceptions.ProviderError):
     """Raised when we can't find the right inventory."""
 
-    def __init__(self, message_data=None):
+    def __init__(self, message=None):
         super().__init__(
             provider="AnsibleTower",
-            message=json.dumps(message_data, indent=2),
+            message=message,
         )
 
 
@@ -161,7 +161,7 @@ def __init__(self, **kwargs):
         # Init the class itself
         config = kwargs.get("config")
         root = kwargs.get("root")
-        self.v2, self.username = get_awxkit_and_uname(
+        self._v2, self.username = get_awxkit_and_uname(
             config=config,
             root=root,
             url=self.url,
@@ -170,7 +170,7 @@ def __init__(self, **kwargs):
             pword=self.pword,
         )
         # Check to see if we're running AAP (ver 4.0+)
-        self._is_aap = self.v2.ping.get().version[0] != "3"
+        self._is_aap = self._v2.ping.get().version[0] != "3"
 
     @staticmethod
     def _pull_params(kwargs):
@@ -217,14 +217,14 @@ def _set_attributes(self, host_inst, broker_args=None, misc_attrs=None):
 
     def _translate_inventory(self, inventory):
         if isinstance(inventory, int):  # already an id, silly
-            if inventory_info := self.v2.inventory.get(id=inventory):
+            if (inventory_info := self._v2.inventory.get(id=inventory)).results:
                 return inventory_info.results[0].name
             else:
                 raise ATInventoryError(
                     message=f"Unknown AnsibleTower inventory by id {inventory}",
                 )
         elif isinstance(inventory, str):
-            if inventory_info := self.v2.inventory.get(search=inventory):
+            if inventory_info := self._v2.inventory.get(search=inventory):
                 if inventory_info.count > 1:
                     # let's try to manually narrow down to one result if the api returns multiple
                     filtered = [inv for inv in inventory_info.results if inv.name == inventory]
@@ -296,7 +296,7 @@ def _merge_artifacts(self, at_object, strategy="last", artifacts=None):
                 if child.type == "workflow_job_node":
                     logger.debug(child)
                     child_id = child.summary_fields.job.id
-                    child_obj = self.v2.jobs.get(id=child_id).results
+                    child_obj = self._v2.jobs.get(id=child_id).results
                     if child_obj:
                         child_obj = child_obj.pop()
                         artifacts = (
@@ -325,7 +325,7 @@ def _get_failure_messages(self, workflow):
                 if child.type == "workflow_job_node":
                     logger.debug(child)
                     child_id = child.summary_fields.job.id
-                    child_obj = self.v2.jobs.get(id=child_id).results
+                    child_obj = self._v2.jobs.get(id=child_id).results
                     if child_obj:
                         child_obj = child_obj.pop()
                         if child_obj.status == "error":
@@ -368,7 +368,11 @@ def _get_failure_messages(self, workflow):
             return failure_messages
 
     def _compile_host_info(self, host):
-        host_facts = host.related.ansible_facts.get()
+        try:
+            host_facts = host.related.ansible_facts.get()
+        except awxkit.exceptions.Forbidden as err:
+            logger.warning(f"Unable to get facts for {host.name}: {err}")
+            host_facts = {}
 
         # Get the hostname from host variables or facts
         hostname = (
@@ -411,10 +415,11 @@ def _get_broker_args_from_job(self, host):
         _broker_args = {}
 
         try:
-            create_job = self.v2.jobs.get(id=host.get_related("job_events").results[0].job)
+            create_job = self._v2.jobs.get(id=host.get_related("job_events").results[0].job)
             create_job = create_job.results[0].get_related("source_workflow_job")
             _broker_args["workflow"] = create_job.name
-        except (IndexError, awxkit.exceptions.Unknown):  # Unknown is a Gateway Timeout
+        # Unknown is a Gateway Timeout
+        except (IndexError, awxkit.exceptions.Unknown, awxkit.exceptions.Forbidden):
             if "last_job" in host.related:
                 # potentially not create job, but easier processing below
                 create_job = host.get_related("last_job")
@@ -458,14 +463,14 @@ def _resolve_labels(self, labels, target):
         for label in labels:
             label_expanded = f"{label}={labels[label]}" if labels[label] else label
             try:
-                result = self.v2.labels.post(
+                result = self._v2.labels.post(
                     {"name": label_expanded, "organization": target.summary_fields.organization.id}
                 )
                 if result:
                     label_ids.append(result.id)
             except awxkit.exceptions.Duplicate:
                 logger.debug(f"Provider label {label_expanded} already exists on AAP instance")
-                if result := self.v2.labels.get(name=label_expanded).results:
+                if result := self._v2.labels.get(name=label_expanded).results:
                     logger.debug(f"Provider label {label_expanded} retrieved successfully")
                     label_ids.append(result[0].id)
                 else:
@@ -583,14 +588,14 @@ def execute(self, **kwargs):  # noqa: PLR0912,PLR0915 - Possible TODO refactor
         """
         if name := kwargs.get("workflow"):
             subject = "workflow"
-            get_path = self.v2.workflow_job_templates
+            get_path = self._v2.workflow_job_templates
             origin = find_origin()
             kwargs["_broker_origin"] = origin[0]
             if origin[1]:
                 kwargs["_jenkins_url"] = origin[1]
         elif name := kwargs.get("job_template"):
             subject = "job_template"
-            get_path = self.v2.job_templates
+            get_path = self._v2.job_templates
         else:
             raise exceptions.UserError(message="No workflow or job template specified")
         try:
@@ -650,7 +655,7 @@ def execute(self, **kwargs):  # noqa: PLR0912,PLR0915 - Possible TODO refactor
                 "URL": job_ui_url,
             }
             helpers.emit(message_data)
-            raise JobExecutionError(message_data=message_data)
+            raise JobExecutionError(message_data=message_data["Reason(s)"])
         if strategy := kwargs.pop("artifacts", None):
             return self._merge_artifacts(job, strategy=strategy)
         return job
@@ -660,7 +665,7 @@ def get_inventory(self, user=None):
         user = user or self.username
         invs = [
             inv
-            for inv in self.v2.inventory.get(page_size=100).results
+            for inv in self._v2.inventory.get(page_size=100).results
             if user in inv.name or user == "@ll"
         ]
         hosts = []
@@ -703,8 +708,8 @@ def provider_help(
         """Get a list of extra vars and their defaults from a workflow."""
         results_limit = kwargs.get("results_limit", settings.ANSIBLETOWER.results_limit)
         if workflow:
-            wfjt = self.v2.workflow_job_templates.get(name=workflow).results.pop()
-            default_inv = self.v2.inventory.get(id=wfjt.inventory).results.pop()
+            wfjt = self._v2.workflow_job_templates.get(name=workflow).results.pop()
+            default_inv = self._v2.inventory.get(id=wfjt.inventory).results.pop()
             logger.info(
                 f"\nDescription:\n{wfjt.description}\n\n"
                 f"Accepted additional nick fields:\n{helpers.yaml_format(wfjt.extra_vars)}"
@@ -713,7 +718,7 @@ def provider_help(
         elif workflows:
             workflows = [
                 workflow.name
-                for workflow in self.v2.workflow_job_templates.get(page_size=1000).results
+                for workflow in self._v2.workflow_job_templates.get(page_size=1000).results
                 if workflow.summary_fields.user_capabilities.get("start")
             ]
             if res_filter := kwargs.get("results_filter"):
@@ -722,19 +727,19 @@ def provider_help(
             workflows = "\n".join(workflows[:results_limit])
             logger.info(f"Available workflows:\n{workflows}")
         elif inventory:
-            inv = self.v2.inventory.get(name=inventory, kind="").results.pop()
+            inv = self._v2.inventory.get(name=inventory, kind="").results.pop()
             inv = {"Name": inv.name, "ID": inv.id, "Description": inv.description}
             logger.info(f"Accepted additional nick fields:\n{helpers.yaml_format(inv)}")
         elif inventories:
-            inv = [inv.name for inv in self.v2.inventory.get(kind="", page_size=1000).results]
+            inv = [inv.name for inv in self._v2.inventory.get(kind="", page_size=1000).results]
             if res_filter := kwargs.get("results_filter"):
                 inv = eval_filter(inv, res_filter, "res")
                 inv = inv if isinstance(inv, list) else [inv]
             inv = "\n".join(inv[:results_limit])
             logger.info(f"Available Inventories:\n{inv}")
         elif job_template:
-            jt = self.v2.job_templates.get(name=job_template).results.pop()
-            default_inv = self.v2.inventory.get(id=jt.inventory).results.pop()
+            jt = self._v2.job_templates.get(name=job_template).results.pop()
+            default_inv = self._v2.inventory.get(id=jt.inventory).results.pop()
             logger.info(
                 f"\nDescription:\n{jt.description}\n\n"
                 f"Accepted additional nick fields:\n{helpers.yaml_format(jt.extra_vars)}"
@@ -743,7 +748,7 @@ def provider_help(
         elif job_templates:
             job_templates = [
                 job_template.name
-                for job_template in self.v2.job_templates.get(page_size=1000).results
+                for job_template in self._v2.job_templates.get(page_size=1000).results
                 if job_template.summary_fields.user_capabilities.get("start")
             ]
             if res_filter := kwargs.get("results_filter"):

tests/functional/test_satlab.py

@@ -135,6 +135,6 @@ def test_tower_provider_labels():
         assert r_host.provider_labels.get("l1") == "v1"
         assert r_host.provider_labels.get("l2") == ""
         # assert the AAP labels got created on the provider
-        aap_labels = [l.name for l in r_host._prov_inst.v2.labels.get().results]
+        aap_labels = [l.name for l in r_host._prov_inst._v2.labels.get().results]
         assert "l1=v1" in aap_labels
         assert "l2" in aap_labels