o-nikolas commented on code in PR #30637:
URL: https://github.com/apache/airflow/pull/30637#discussion_r1167325204
##########
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:
This is nice, simplifies the logic quite a bit.
> shall we initialise cancel rather than hardcoding it, like we have others
states?
It's only one item, but on the other hand `FAILED_STATES` is only one item
and we have a set for it. So I'd +1 doing the same for cancelled for
consistency sake.
--
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]
