[GitHub] [airflow] vandonr-amz commented on a diff in pull request #28869: rewrite polling code for appflow hook

["vandonr-amz (via GitHub)" <gi...@apache.org>]

vandonr-amz commented on code in PR #28869:
URL: https://github.com/apache/airflow/pull/28869#discussion_r1123512646


##########
airflow/providers/amazon/aws/hooks/appflow.py:
##########
@@ -54,49 +50,35 @@ def conn(self) -> AppflowClient:
         """Get the underlying boto3 Appflow client (cached)"""
         return super().conn
 
-    def run_flow(self, flow_name: str, poll_interval: int = 20) -> str:
+    def run_flow(self, flow_name: str, poll_interval: int = 20, wait_for_completion: bool = True) -> str:
         """
         Execute an AppFlow run.
 
         :param flow_name: The flow name
         :param poll_interval: Time (seconds) to wait between two consecutive calls to check the run status
+        :param wait_for_completion: whether to wait for the run to end to return
         :return: The run execution ID
         """
-        ts_before: datetime = datetime.now(timezone.utc)
-        sleep(self.EVENTUAL_CONSISTENCY_OFFSET)
         response_start = self.conn.start_flow(flowName=flow_name)
         execution_id = response_start["executionId"]
         self.log.info("executionId: %s", execution_id)
 
-        response_desc = self.conn.describe_flow(flowName=flow_name)
-        last_exec_details = response_desc["lastRunExecutionDetails"]
-
-        # Wait Appflow eventual consistence
-        self.log.info("Waiting for Appflow eventual consistence...")
-        while (
-            response_desc.get("lastRunExecutionDetails", {}).get(
-                "mostRecentExecutionTime", datetime(1970, 1, 1, tzinfo=timezone.utc)
-            )
-            < ts_before
-        ):
-            sleep(self.EVENTUAL_CONSISTENCY_POLLING)
-            response_desc = self.conn.describe_flow(flowName=flow_name)
-            last_exec_details = response_desc["lastRunExecutionDetails"]
-
-        # Wait flow stops
-        self.log.info("Waiting for flow run...")
-        while (
-            "mostRecentExecutionStatus" not in last_exec_details
-            or last_exec_details["mostRecentExecutionStatus"] == "InProgress"
-        ):
-            sleep(poll_interval)
-            response_desc = self.conn.describe_flow(flowName=flow_name)
-            last_exec_details = response_desc["lastRunExecutionDetails"]
-
-        self.log.info("lastRunExecutionDetails: %s", last_exec_details)
-
-        if last_exec_details["mostRecentExecutionStatus"] == "Error":
-            raise Exception(f"Flow error:\n{json.dumps(response_desc, default=str)}")
+        if wait_for_completion:

Review Comment:
   ok, but do you think it should be done as part of this PR ? How about I push the conversion to waiter as a new PR ?



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org