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-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
