o-nikolas commented on code in PR #30466:
URL: https://github.com/apache/airflow/pull/30466#discussion_r1157677141
##########
airflow/providers/amazon/aws/hooks/quicksight.py:
##########
@@ -113,10 +113,17 @@ def get_status(self, aws_account_id: str, data_set_id:
str, ingestion_id: str) -
AwsAccountId=aws_account_id, DataSetId=data_set_id,
IngestionId=ingestion_id
)
return describe_ingestion_response["Ingestion"]["IngestionStatus"]
- except KeyError:
- raise AirflowException("Could not get status of the Amazon
QuickSight Ingestion")
- except ClientError:
- raise AirflowException("AWS request failed, check logs for more
info")
+ except KeyError as e:
+ raise AirflowException(f"Could not get status of the Amazon
QuickSight Ingestion: {e}")
+ except ClientError as e:
+ raise AirflowException(f"AWS request failed: {e}")
+
+ def get_error_info(self, aws_account_id: str, data_set_id: str,
ingestion_id: str) -> dict | None:
+ """If the ingestion failed, returns the error info. Else, returns
None."""
Review Comment:
Do you mind reformatting this with `param <param_name>:` and `return:` tags?
The rest of the module follows that quite closely
##########
airflow/providers/amazon/aws/hooks/quicksight.py:
##########
@@ -113,10 +113,17 @@ def get_status(self, aws_account_id: str, data_set_id:
str, ingestion_id: str) -
AwsAccountId=aws_account_id, DataSetId=data_set_id,
IngestionId=ingestion_id
)
return describe_ingestion_response["Ingestion"]["IngestionStatus"]
- except KeyError:
- raise AirflowException("Could not get status of the Amazon
QuickSight Ingestion")
- except ClientError:
- raise AirflowException("AWS request failed, check logs for more
info")
+ except KeyError as e:
+ raise AirflowException(f"Could not get status of the Amazon
QuickSight Ingestion: {e}")
+ except ClientError as e:
+ raise AirflowException(f"AWS request failed: {e}")
+
+ def get_error_info(self, aws_account_id: str, data_set_id: str,
ingestion_id: str) -> dict | None:
+ """If the ingestion failed, returns the error info. Else, returns
None."""
+ describe_ingestion_response = self.get_conn().describe_ingestion(
+ AwsAccountId=aws_account_id, DataSetId=data_set_id,
IngestionId=ingestion_id
+ )
+ return describe_ingestion_response["Ingestion"].get("ErrorInfo", None)
Review Comment:
Nit: the default of `.get(...)` is already None.
##########
airflow/providers/amazon/aws/hooks/quicksight.py:
##########
@@ -141,12 +148,14 @@ def wait_for_state(
status = self.get_status(aws_account_id, data_set_id, ingestion_id)
while status in self.NON_TERMINAL_STATES and status != target_state:
self.log.info("Current status is %s", status)
- time.sleep(check_interval)
- sec += check_interval
if status in self.FAILED_STATES:
- raise AirflowException("The Amazon QuickSight Ingestion
failed!")
+ 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!")
+ # wait and try again
+ time.sleep(check_interval)
+ sec += check_interval
Review Comment:
:+1:
--
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]
