Subham-KRLX commented on PR #64945:
URL: https://github.com/apache/airflow/pull/64945#issuecomment-4358058686
> This is almost certainly wrong / hallucinated:
>
> a) tests are in airflow-core, where fix is in task-sdk b) the error that
is happening when context is set is 500 which is different than the error in
the issue (not found) c) when you debug it you see that one of the things that
happens in `task_runner.py` is that an exception is raised in exception
handling routine :
>
> ```python
> except DagRunTriggerException as drte:
> msg, state = _handle_trigger_dag_run(drte, context, ti, log)
> ```
>
> `_handle_trigger_dag_run` raises an exception on it's own and it causes
the underlying finally, does not have `state` set:
>
> ```python
> finally:
> Stats.incr(
> f"ti.finish.{ti.dag_id}.{ti.task_id}.{state.value}",
> tags=stats_tags,
> )
> # Same metric with tagging
> Stats.incr("ti.finish", tags={**stats_tags, "state": state.value})
> ```
>
> And the problem is that `state` variable had never been set because we
have exception thrown while we are handling DagRunTriggerException - because
there is another COMMS failure when task_run tries to send something to
supervisor and fails - without handling the COMMS exception.
>
> @kaxil @dstandish @amoghrajesh @ashb -> since this is a crucial part of
"task run" - I think it should be looked at - I do not know that part of the
code after task-sdk split, but this feels quite wrong that exception handling
as it is now allows to end up with unitialized msg/state variables and using
state in finally. It feels that we have somewhat wrong pattern of exception
handling implemented here (but I am not sure how it should be).
>
> It's easy to repro - just take the tests that @Subham-KRLX implemented and
revert removal of `_airflow_parsing_context_manager`.
You are right the dag.py change was wrong I will revert it and fix the
actual root cause in task_runner.py where state can be uninitialized in the
finally block the tests will stay as they correctly reproduce the bug.
--
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]
