[GitHub] [airflow] jedcunningham commented on a diff in pull request #29832: Add `_on_failure_fail_dagrun` attribute for teardown tasks

["jedcunningham (via GitHub)" <gi...@apache.org>]

jedcunningham commented on code in PR #29832:
URL: https://github.com/apache/airflow/pull/29832#discussion_r1123432211


##########
airflow/utils/setup_teardown.py:
##########
@@ -43,15 +43,19 @@ def setup(cls):
 
     @classmethod
     @contextmanager
-    def teardown(cls):
+    def teardown(cls, on_failure_fail_dagrun=None):
         if cls.is_setup or cls.is_teardown:
             raise AirflowException(
                 "A teardown task or taskgroup cannot be nested inside another"
                 " setup/teardown task or taskgroup"
             )
 
         cls.is_teardown = True
+        if on_failure_fail_dagrun:
+            setattr(cls, "on_failure_fail_dagrun", on_failure_fail_dagrun)

Review Comment:
   We should probably always have this attr in class as false, and simply toggle it true.



##########
airflow/utils/task_group.py:
##########
@@ -245,6 +245,8 @@ def add(self, task: DAGNode) -> None:
         elif SetupTeardownContext.is_teardown:
             if isinstance(task, AbstractOperator):
                 setattr(task, "_is_teardown", True)
+                if getattr(SetupTeardownContext, "on_failure_fail_dagrun", False):
+                    setattr(task, "_on_failure_fail_dagrun", True)

Review Comment:
   Same here, we shouldn't just graft this in, but add a false default to BaseOperator. Should probably do the same for _is_setup/_is_teardown.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org