[GitHub] [airflow] PhilipGarnero commented on a change in pull request #5083: [AIRFLOW-4292] Cleanup and improve SLA code

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services