XD-DENG commented on a change in pull request #5065: [AIRFLOW-4261] Minor
refactoring on jobs.py
URL: https://github.com/apache/airflow/pull/5065#discussion_r273427830
##########
File path: airflow/jobs.py
##########
@@ -665,8 +665,7 @@ def manage_slas(self, dag, session=None):
slas = (
session
.query(SlaMiss)
- .filter(SlaMiss.notification_sent == False) # noqa: E712
- .filter(SlaMiss.dag_id == dag.dag_id)
+ .filter(SlaMiss.notification_sent == False, SlaMiss.dag_id ==
dag.dag_id) # noqa: E712
Review comment:
In the benchmarking in https://github.com/apache/airflow/pull/4433 you can
find that:
- `.filter(DM.is_active == True)` and `.filter(DM.is_active)` are very
different in terms of performance. I think the main difference is in Python
code side rather than DB side (extra calculation of ` == True`).
- `.filter(cond_1).filter(cond_2)` and `.filter(cond_1, cond_2)` are very
close. So this change here in this PR is not really brining that big
improvement.
I used both _MariaDB_ and _SQLite_ for the benchmarking in
https://github.com/apache/airflow/pull/4433 (not sure which one it is for the
figures shared. But I remember the results from two DBs are consistent)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
