ferruzzi commented on code in PR #64085:
URL: https://github.com/apache/airflow/pull/64085#discussion_r2977946099


##########
providers/amazon/src/airflow/providers/amazon/aws/operators/neptune.py:
##########
@@ -187,14 +188,13 @@ def execute(self, context: Context, event: dict[str, Any] 
| None = None, **kwarg
         return {"db_cluster_id": self.cluster_id}
 
     def execute_complete(self, context: Context, event: dict[str, Any] | None 
= None) -> dict[str, str]:
-        status = ""
-        cluster_id = ""
+        validated_event = validate_execute_complete_event(event)
 
-        if event:
-            status = event.get("status", "")
-            cluster_id = event.get("cluster_id", "")
+        if validated_event["status"] != "success":
+            raise AirflowException(f"Error starting Neptune cluster: 
{validated_event}")
 
-        self.log.info("Neptune cluster %s available with status: %s", 
cluster_id, status)

Review Comment:
   Is there a reason to drop `status` from the log here (and int he similar 
logs below)?



##########
providers/amazon/src/airflow/providers/amazon/aws/triggers/base.py:
##########
@@ -149,13 +150,17 @@ async def run(self) -> AsyncIterator[TriggerEvent]:
                 client=client,
                 config_overrides=self.waiter_config_overrides,
             )
-            await async_wait(
-                waiter,
-                self.waiter_delay,
-                self.attempts,
-                self.waiter_args,
-                self.failure_message,
-                self.status_message,
-                self.status_queries,
-            )
-            yield TriggerEvent({"status": "success", self.return_key: 
self.return_value})
+            try:
+                await async_wait(
+                    waiter,
+                    self.waiter_delay,
+                    self.attempts,
+                    self.waiter_args,
+                    self.failure_message,
+                    self.status_message,
+                    self.status_queries,
+                )
+            except AirflowException as e:
+                yield TriggerEvent({"status": "error", "message": str(e), 
self.return_key: self.return_value})

Review Comment:
   You don't see many try/except/else blocks.  I like it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to