[GitHub] [airflow] uranusjr commented on a change in pull request #18208: Celery: Sort adopted tasks in _check_for_stalled_adopted_tasks method

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

uranusjr commented on a change in pull request #18208:
URL: https://github.com/apache/airflow/pull/18208#discussion_r708876286



##########
File path: airflow/executors/celery_executor.py
##########
@@ -341,11 +341,13 @@ def _check_for_stalled_adopted_tasks(self):
         just resend them. We do that by clearing the state and letting the
         normal scheduler loop deal with that
         """
-        now = utcnow()
+        sorted_adopted_task_timeouts = OrderedDict(
+            sorted(self.adopted_task_timeouts.items(), key=lambda k: k[1])
+        )

Review comment:
       Probably don’t even need `OrderedDict` since all we do is calling `items()` later anyway.
   
   This should be good enough:
   
   ```
   sorted_adopted_task_timeouts = sorted(self.adopted_task_timeouts.items(), key=lambda k: k[1])
   
   for key, stalled_after in sorted_adopted_task_timeouts:
       ...
   ```
   
   Also I feel having `utcnow()` outside of the loop is slightly better? It saves some function calls, unless millisecond accuracy is actually important here. But that’s minor either way.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org