Don't clear downstream tasks when marked status is failure and downstream is True

[tirkarthi]

• Don't clear status when a task is marked as failure with downstream selected since marking it failure and again clearing status will re-run the tasks again.
• When a task that is already failed is again marked as failure even without downstream not being selected then don't clear the status of downstream tasks.

closes: #22995
related: #22995

[tirkarthi]

Test case and end result for the change.

 with DAG("test_mark_task_instance_state_failed_downstream_clear", start_date=start_date) as dag:
    task_1 = EmptyOperator(task_id="task_1")  
    task_2 = EmptyOperator(task_id="task_2")
    task_3 = EmptyOperator(task_id="task_3")                                                                                                                          

    task_1 >> task_2 >> task_3 

Mark task_1 as Failed with Downstream True

Task	Start state	End state
task_1	Success	Failed
task_2	Success	Failed
task_3	Success	Failed

Task	Start state	End state
task_1	Success	Failed
task_2	Success	Failed
task_3	Failed	Failed

Mark task_1 as Failed with Downstream False

Task	Start state	End state
task_1	Success	Failed
task_2	Success	Success
task_3	Failed	None

Task	Start state	End state
task_1	Failed	Failed
task_2	Success	Success
task_3	Failed	Failed

Task	Start state	End state
task_1	Success	Failed
task_2	Success	Success
task_3	Success	Success

[ashb]

Haven't looked at tests (I'm on mobile so I can't read them easily) but the code change makes sense

[jedcunningham]

Why does this one become none?

[tirkarthi]

This is to ensure the behavior in #13037 is kept in place. I also find it slightly counter intuitive since downstream is not selected and marking a task as failure shouldn't clear the state of downstream tasks that are failed already for re-run. Should we change it so that failed state is preserved?

[bbovenzi]

Testing locally. This looks good to me.

[github-actions]

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

[ashb]

This doesn't seem right. task2 and task3 should either be left as success, or changed to upstream failed. I don't think failed is right though...

[tirkarthi]

Current behavior is that setting a task instance as Failure with downstream=True results in clearing the downstream tasks that are rerun to be set as upstream failed. The reported issue #22995 expects the downstream tasks also to be set as failure rather than being cleared for re-run.

The behavior was introduced in #13037 where task marked as success cleared downstream tasks for re-run. When the task is marked as failure with downstream=True should we clear for re-run or should we set it to failure?

[jedcunningham]

My 2c is it needs to be consistent. If we like the "success + downstream" clearing downstream behavior, it should do the same thing with "failed + downstream".

So I think the current behavior is correct, but I think the dialog could be refactored to indicate that the downstreams are cleared.

And here is a reason you'd want clearing instead of just setting everything to failed. What if you have tasks with one_failed or all_failed as a trigger rule downstream? Forcing everything downstream as failed assumes all_success...

[ashb]

Why is this in test_views.py? All the mark functions in views.py do is call dag.set_task_instance_state and then redirect. Nothing should be tested in views for this change.

[tirkarthi]

There is test_mark_task_instance_state which had tests for marking instance state. I thought to add them here since the original issue was about marking it through API. Do you want me to move it to tests/models/test_dag.py ?

[itstalmeez]

Resolves issue #22995 by preventing the clearing of task status when marking a task as failed with downstream selected. This adjustment is crucial to avoid unintended task reruns. Previously, marking a task as failed with downstream selected resulted in both marking it as failed and subsequently clearing its status, causing unnecessary task reruns.

Additionally, in cases where a task is already in a failed state and is marked as failed again without downstream selected, this fix ensures that the status of downstream tasks is not cleared. This behavior ensures consistency in handling task status, preventing unnecessary resets when not explicitly intended.

Your feedback on this improvement is appreciated. Please review and share any insights. This change is closely related to the ongoing efforts to enhance task status management, as noted in issue #22995. Thank you for your attention to these considerations.

[potiuk]

Resolves issue #22995 by preventing the clearing of task status when marking a task as failed with downstream selected. This adjustment is crucial to avoid unintended task reruns. Previously, marking a task as failed with downstream selected resulted in both marking it as failed and subsequently clearing its status, causing unnecessary task reruns.

Additionally, in cases where a task is already in a failed state and is marked as failed again without downstream selected, this fix ensures that the status of downstream tasks is not cleared. This behavior ensures consistency in handling task status, preventing unnecessary resets when not explicitly intended.

Your feedback on this improvement is appreciated. Please review and share any insights. This change is closely related to the ongoing efforts to enhance task status management, as noted in issue #22995. Thank you for your attention to these considerations.

Please stop posting chat-gpt / AI generated comments. They are nonsencial, useless and bring no value for anyone @itstalmeez . I was tempted to report you to Github which might result in your account being suspended, but for a while I decided to hold on to see if warning like that will be efficient.

This adds a lot of noise to communication here and you are spamming our project. One more time and report will be sent.

[bbovenzi]

Rebase needed.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.