Subham-KRLX commented on PR #64945: URL: https://github.com/apache/airflow/pull/64945#issuecomment-4360394134
> Wrong fix @Subham-KRLX because this doesn't actually fix #64884. With only the state = TaskInstanceState.FAILED default in task_runner.py, calling parent.test() still: > > 1. Skips the bundle sync in DAG.test() whenever `parent_dag` is already in the DB (the if not version: guard at `task-sdk/src/airflow/sdk/definitions/dag.py:1295`), so `other_dag` is never > synced. > 2. Trigger still hits the in-process API and gets a 404 for `other_dag`. > 3. `_handle_trigger_dag_run` still raises `ServerResponseError` straight out of the except `DagRunTriggerException` block. > 4. Task ends up in failed state, the `dag.test()` run state is failed, not success. > > The reporter's assert `run.state == State.SUCCESS` would still fail. The included tests in this PR also assert `dr.state == DagRunState.SUCCESS` and that the target run was created, neither of which holds without something actually syncing the target DAG. > > The `state = FAILED` default is a band-aid that just stops the UnboundLocalError in the finally and silently labels every unhandled-exception path as FAILED with a misleading metric. I will look at fixing the bundle sync in dag.test() properly and handling the ErrorResponse for non DAGRUN_ALREADY_EXISTS errors in _handle_trigger_dag_run. Will update once I have a proper fix. -- 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]
