SameerMesiah97 commented on code in PR #61333:
URL: https://github.com/apache/airflow/pull/61333#discussion_r2753582483
##########
providers/amazon/src/airflow/providers/amazon/aws/operators/redshift_cluster.py:
##########
@@ -188,6 +192,7 @@ def __init__(
max_attempt: int = 5,
poll_interval: int = 60,
deferrable: bool = conf.getboolean("operators", "default_deferrable",
fallback=False),
+ delete_cluster_on_failure: bool = True,
Review Comment:
> I don't think this is needed? Airflow has teardown feature
https://airflow.apache.org/docs/apache-airflow/stable/howto/setup-and-teardown.html#setup-and-teardown
That is a perfectly valid point. But I believe this new flag handles
clean-ups slightly differently than teardowns. It aims to prevent resource
leaks as a result of a class of failures caused by partial IAM permissions,
which users may not anticipate. Users can model cleanup with teardown tasks if
they anticipate these failures, but that requires them to foresee fairly
provider-specific failure modes. But then we are shifting the responsibility of
handling unexpected resource leaks to DAG authors.
My understanding is that providers should handle this class of failure by
default, and that’s what this flag is trying to do, without preventing users
from using teardown for broader DAG-level lifecycle management.
--
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]