vincbeck commented on code in PR #30637:
URL: https://github.com/apache/airflow/pull/30637#discussion_r1167054272
##########
airflow/providers/amazon/aws/hooks/quicksight.py:
##########
@@ -156,16 +156,17 @@ def wait_for_state(
sec = 0
status = self.get_status(aws_account_id, data_set_id, ingestion_id)
while status in self.NON_TERMINAL_STATES and status != target_state:
+ time.sleep(check_interval)
self.log.info("Current status is %s", status)
+ status = self.get_status(aws_account_id, data_set_id, ingestion_id)
if status in self.FAILED_STATES:
info = self.get_error_info(aws_account_id, data_set_id,
ingestion_id)
raise AirflowException(f"The Amazon QuickSight Ingestion
failed. Error info: {info}")
if status == "CANCELLED":
raise AirflowException("The Amazon QuickSight SPICE ingestion
cancelled!")
Review Comment:
I would do it that way to avoid duplicate calls on `self.get_status`
```suggestion
while True:
status = self.get_status(aws_account_id, data_set_id,
ingestion_id)
self.log.info("Current status is %s", status)
if status in self.FAILED_STATES:
info = self.get_error_info(aws_account_id, data_set_id,
ingestion_id)
raise AirflowException(f"The Amazon QuickSight Ingestion
failed. Error info: {info}")
if status == "CANCELLED":
raise AirflowException("The Amazon QuickSight SPICE
ingestion cancelled!")
if status not in self.NON_TERMINAL_STATES or status ==
target_state:
break
time.sleep(check_interval)
```
##########
airflow/providers/amazon/aws/hooks/quicksight.py:
##########
@@ -156,16 +156,17 @@ def wait_for_state(
sec = 0
status = self.get_status(aws_account_id, data_set_id, ingestion_id)
while status in self.NON_TERMINAL_STATES and status != target_state:
+ time.sleep(check_interval)
self.log.info("Current status is %s", status)
+ status = self.get_status(aws_account_id, data_set_id, ingestion_id)
if status in self.FAILED_STATES:
info = self.get_error_info(aws_account_id, data_set_id,
ingestion_id)
raise AirflowException(f"The Amazon QuickSight Ingestion
failed. Error info: {info}")
if status == "CANCELLED":
raise AirflowException("The Amazon QuickSight SPICE ingestion
cancelled!")
Review Comment:
Also, `sec` is not used so it is safe to remove 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]
