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]

Reply via email to