[GitHub] [airflow] Taragolis commented on a diff in pull request #33535: Sort data before groupby in TIS duration calculation

["Taragolis (via GitHub)" <gi...@apache.org>]

Taragolis commented on code in PR #33535:
URL: https://github.com/apache/airflow/pull/33535#discussion_r1299327049


##########
airflow/www/views.py:
##########
@@ -3237,7 +3237,8 @@ def duration(self, dag_id: str, session: Session = NEW_SESSION):
 
         # we must group any mapped TIs by dag_id, task_id, run_id
         mapped_tis = set()
-        tis_grouped = itertools.groupby(task_instances, lambda x: (x.dag_id, x.task_id, x.run_id))
+        tis_group_sort = lambda ti: (ti.dag_id, ti.task_id, ti.run_id)

Review Comment:
   > There’s a rule E731 that advises you to not assign a lambda but use def instead.
   
   Oh, I assume that we deliberately turned off E731. No problem I will wrap it into the generator
   
   > With that said, TaskInstance actually has a function for this so this can just be operator.attrgetter("key").
   
   Might be suitable for sorting, by not for grouping in this case



##########
airflow/www/views.py:
##########
@@ -3237,7 +3237,8 @@ def duration(self, dag_id: str, session: Session = NEW_SESSION):
 
         # we must group any mapped TIs by dag_id, task_id, run_id
         mapped_tis = set()
-        tis_grouped = itertools.groupby(task_instances, lambda x: (x.dag_id, x.task_id, x.run_id))
+        tis_group_sort = lambda ti: (ti.dag_id, ti.task_id, ti.run_id)

Review Comment:
   > There’s a rule E731 that advises you to not assign a lambda but use def instead.
   
   Oh, I assume that we deliberately turned off E731. No problem I will wrap it into the generator
   
   > With that said, TaskInstance actually has a function for this so this can just be operator.attrgetter("key").
   
   Might be suitable for sorting, by not for grouping in this case



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