Send DAG timeout callbacks to processor outside of prohibit_commit #24366
Conversation
boring-cyborg
bot
added
the
area:Scheduler
| @@ -1159,10 +1159,7 @@ def _schedule_dag_run( | |||
| msg='timed_out', | |||
| ) | |||
|
|
|||
| # Send SLA & DAG Success/Failure Callbacks to be executed | |||
| self._send_dag_callbacks_to_processor(dag, callback_to_execute) | |||
There was a problem hiding this comment.
Whoops, didn't this end up with the callback being sent twice before too?
There was a problem hiding this comment.
Yet another case where our session handling causes bugs. Our use of @create_session which does an expunge_all and closes the session when it exits needs fixing.
There was a problem hiding this comment.
No, the callback is executed once, because the callback it returned was None.
github-actions
bot
added
the
full tests needed
|
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. |
|
@mhenc - can you also take a look here seems that standalone dag processor could cause problem with commit inside a "prohibit commit" guard. |
Sending DAG timeout callbacks outside of prohibit_commit because when using standalone_dag_processor they are added to the database.
Issue reported in slack
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragement file, named
{pr_number}.significant.rst, in newsfragments.