-
Notifications
You must be signed in to change notification settings - Fork 13.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add cascade to DagRun/TaskInstance relationship #18434
Conversation
airflow/models/dagrun.py
Outdated
@@ -124,7 +124,7 @@ class DagRun(Base, LoggingMixin): | |||
), | |||
) | |||
|
|||
task_instances = relationship(TI, back_populates="dag_run") | |||
task_instances = relationship(TI, back_populates="dag_run", cascade='all, delete, delete-orphan') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to documentation all
implies delete
so the latter is not needed. However, I don’t think we should do expunge
(also implied by all
). I’m also not sure about refresh-expire
. Maybe a safer route would be save-update, merge, delete, delete-orphan
(the first two are the default value).
b4281ff
to
d6288f6
Compare
When did this start failing? Cos it has been passing for a while. Oh new test added in #16634 that passedon PR but never worked on main :( |
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. |
We currently have issue where deleting dagruns causes a dependency error in Sqlalchemy because the session doesn't know what to do with the related taskinstances. This PR adds cascade so that when a dagrun is marked for deletion, the related taskinstances are also deleted
4d9799c
to
5e6413d
Compare
We currently have an issue where deleting dagruns causes a dependency error in Sqlalchemy because
the session doesn't know what to do with the related taskinstances.
This PR adds cascade so that when a dagrun is marked for deletion, the related taskinstances
are also deleted
^ 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 UPDATING.md.