Retry on Airflow Schedule DAG Run DB Deadlock#26347
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
|
|
You also need to rebase @anthonyp97 to fix unrelated failures I think. |
airflow/jobs/scheduler_job.py
Outdated
|
|
||
| guard.commit() | ||
|
|
||
| return callback_tuples, callback_to_run |
There was a problem hiding this comment.
Is there a reason you're including callback_to_run in the return statement here?? It seems cleaner to just return callback_tuples, especially because the last callback_to_run is going to be included in the last tuple of callback_tuples anyway.
The next line of code overwrites callback_to_run as well, so it just seems unnecessary to pass it back:
callback_tuples, callback_to_run = self._schedule_all_dag_runs(guard, dag_runs, session)
# ...
for dag_run, callback_to_run in callback_tuples: # <-- callback_to_run overwritten immediatelyAlso, would a dictionary be a better option to map dag runs to callbacks?
There was a problem hiding this comment.
@blag yea definitely right that I should just be returning callback_tuples, will update this. For the second point I imagine it shouldn't really matter between list of tuples and a dict since we are just iterating through the data structure either way element by element but if you strongly prefer a dict I can update to use that.
There was a problem hiding this comment.
I don't strongly prefer a dict, that's just what makes more sense to me. But yeah, if a list works then go with that.
|
I think we are just about to release 2.4.1 I think it looks good to me, but we have just merged 2.4.1 branch to release it. I think it will go to 2.4.2. |
|
I also think @ashb should take a look. |
|
Awesome work, congrats on your first merged pull request! |
Co-authored-by: Anthony Panat <anthonypanat@Anthonys-MacBook-Pro-2.local> Co-authored-by: Anthony Panat <anthonypanat@anthonys-mbp-2.mynetworksettings.com> (cherry picked from commit 0da4993)
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an 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 newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.