dstandish commented on a change in pull request #20699:
URL: https://github.com/apache/airflow/pull/20699#discussion_r779646540



##########
File path: airflow/jobs/triggerer_job.py
##########
@@ -380,10 +380,16 @@ def update_triggers(self, requested_trigger_ids: 
Set[int]):
         # line's execution, but we consider that safe, since there's a strict
         # add -> remove -> never again lifecycle this function is already
         # handling.
-        current_trigger_ids = set(self.triggers.keys())
+        running_trigger_ids = set(self.triggers.keys())
+        known_trigger_ids = (
+            running_trigger_ids.union({x[0] for x in self.events})
+            .union(self.to_cancel)
+            .union({x[0] for x in self.to_create})
+            .union(self.failed_triggers)
+        )

Review comment:
       re
   
   > .union(x[0] for x in self.to_create)
   
   that's pretty cool, i don't recall ever noticing that you can do that that.  
 never occurred to me that the "comprehension" part emits a generator and is 
distinct from the "list" part.
   
   but it's interesting, there is  still something "magical" that happens when 
you put the generator expression in the list (or braces)
   
   maybe a bit surprising you can't do the below, given how it seems mosly the 
same as `a if x else b`
   
   ```
   a = x for x in [1,2,3]
   ```
   
   but in any case this is allowed
   
   ```
   a = (x for x in [1,2,3]) # generator
   [a] # list of generator
   ```
   
   but of course is not the same as this
   
   ```
   [x for x in [1,2,3]] # list of int
   ```
   
   anyway, cleaner this way -- thanks




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to