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
Clear tasks by task ids in REST API #14500
Clear tasks by task ids in REST API #14500
Conversation
@@ -1227,6 +1230,8 @@ def clear( | |||
tis = tis.filter(or_(TI.state == State.FAILED, TI.state == State.UPSTREAM_FAILED)) | |||
if only_running: | |||
tis = tis.filter(TI.state == State.RUNNING) | |||
if task_ids: | |||
tis = tis.filter(TI.task_id.in_(task_ids)) |
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.
How this will work with
tis = tis.filter(TI.task_id.in_(self.task_ids))
from L1197?
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.
Actually the conditions are just added with conjunction:
# tst_double_in_query.py
from sqlalchemy.orm import Session
from airflow.models import TaskInstance
session = Session()
query = session.query(TaskInstance.task_id).filter(TaskInstance.task_id.in_(["foo"]))
print(str(query.statement))
query = query.filter(TaskInstance.task_id.in_(["bar"]))
print(str(query.statement))
$ python tst_double_in_query.py
First query:
SELECT task_instance.task_id
FROM task_instance
WHERE task_instance.task_id IN (:task_id_1)
Second query:
SELECT task_instance.task_id
FROM task_instance
WHERE task_instance.task_id IN (:task_id_1) AND task_instance.task_id IN (:task_id_2)
So the second filter narrows down the search space if task_ids
are provided.
Naturally we can intersect the sets preliminary and apply the filter once, it can make the generated SQL code a little more efficient. Would it be better, how do you feel?
I'm not sure whether we should check provided |
0c145fd
to
2ccfb95
Compare
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.
LGTM @mik-laj @turbaszek
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 master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease. |
Please rebase and push |
2801f9c
to
005193d
Compare
005193d
to
74106e3
Compare
@kaxil @ephraimbuddy gentle ping |
Closing and reopening to trigger test runs |
74106e3
to
fd6e690
Compare
closes: #13225