XD-DENG commented on a change in pull request #12815:
URL: https://github.com/apache/airflow/pull/12815#discussion_r536305168



##########
File path: airflow/models/dagrun.py
##########
@@ -718,19 +718,20 @@ def schedule_tis(self, schedulable_tis: Iterable[TI], 
session: Session = None) -
         All the TIs should belong to this DagRun, but this code is in the 
hot-path, this is not checked -- it
         is the caller's responsibility to call this function only with TIs 
from a single dag run.
         """
-        # Get list of TIs that do not need to executed, these are
+        # Get list of TI IDs that do not need to executed, these are
         # tasks using DummyOperator and without on_execute_callback / 
on_success_callback
-        dummy_tis = {
-            ti
-            for ti in schedulable_tis
+        dummy_ti_ids = []
+        schedulable_ti_ids = []
+        for ti in schedulable_tis:
             if (
                 ti.task.inherits_from_dummy_operator
                 and not ti.task.on_execute_callback
                 and not ti.task.on_success_callback
-            )
-        }
+            ):
+                dummy_ti_ids.append(ti.task_id)
+            else:
+                schedulable_ti_ids.append(ti.task_id)
 
-        schedulable_ti_ids = [ti.task_id for ti in schedulable_tis if ti not 
in dummy_tis]
         count = 0

Review comment:
       Personally I don't find it necessary (for now).
   
   On the other hand, if we abstract this into a separate function, at least to 
what I can see, it's sort of "duplicated" with `helpers.partition()` (I don't 
want to use `helpers.partition()` here because it still traverse the iterable 
twice.)




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


Reply via email to