PhilipGarnero commented on a change in pull request #5083: [AIRFLOW-4292] 
Cleanup and improve SLA code
URL: https://github.com/apache/airflow/pull/5083#discussion_r284182208
 
 

 ##########
 File path: airflow/jobs.py
 ##########
 @@ -622,6 +622,7 @@ def manage_slas(self, dag, session=None):
             self.log.info("Skipping SLA check for %s because no tasks in DAG 
have SLAs", dag)
             return
 
+        # Find the last executed tasks
 
 Review comment:
   If I'm not mistaken, this is not find the last executed tasks but rather, 
find the last succesful or skipped task instances.
   Which by the way is a bit weird to base SLAs from only successful tasks 
instances. What if the the whole history has failed ? We don't get any SLA ? I 
think we need to change the filter down below too so it uses any task that 
hasn't be scheduled yet as a basis.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to