amoghrajesh commented on code in PR #69874:
URL: https://github.com/apache/airflow/pull/69874#discussion_r3584790415
##########
airflow-core/src/airflow/api/common/mark_tasks.py:
##########
Review Comment:
What about teardown task instances that are left running by design (so they
can finish their own cleanup)? They are still included in the `running_tis`
list returned by `_set_dag_run_terminal_state`, so the API server will fire a
false "success"/"failed" terminal event for a teardown task that is, in fact,
still executing.
This might've been an untested edge case even before this PR but still
flagging.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_run.py:
##########
@@ -1609,6 +1609,70 @@ def test_patch_dag_run_notifies_listeners(self,
test_client, state, listener_sta
assert response.status_code == 200
assert listener.state == listener_state
+ @pytest.mark.parametrize(
+ ("dag_run_state", "expected_ti_listener_state"),
+ [
+ ("success", TaskInstanceState.SUCCESS),
+ ("failed", TaskInstanceState.FAILED),
+ ],
+ )
+ @pytest.mark.usefixtures("configure_git_connection_for_dag_bundle")
+ def test_patch_dag_run_notifies_ti_listeners_for_running_tasks(
+ self,
+ test_client,
+ dag_maker,
+ session,
+ listener_manager,
+ dag_run_state,
+ expected_ti_listener_state,
+ ):
Review Comment:
We should also cover for teardown tasks, both here and in the other test.
--
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]
