uranusjr commented on code in PR #24902:
URL: https://github.com/apache/airflow/pull/24902#discussion_r923044370


##########
airflow/models/dag.py:
##########
@@ -2120,6 +2120,13 @@ def filter_task_group(group, parent_group):
     def has_task(self, task_id: str):
         return task_id in self.task_dict
 
+    def has_task_group(self, task_group_id: str):
+        return task_group_id in self.task_group_dict

Review Comment:
   ```suggestion
       def has_task_group(self, task_group_id: str) -> bool:
           return task_group_id in self.task_group_dict
   ```



##########
airflow/sensors/external_task.py:
##########
@@ -143,18 +151,25 @@ def __init__(
         if external_task_id is not None:
             external_task_ids = [external_task_id]
 
-        if external_task_ids:
+        if external_task_group_id and external_task_ids:

Review Comment:
   This can use `exactly_one` and also should use `is None`.



##########
airflow/models/dag.py:
##########
@@ -2120,6 +2120,13 @@ def filter_task_group(group, parent_group):
     def has_task(self, task_id: str):
         return task_id in self.task_dict
 
+    def has_task_group(self, task_group_id: str):
+        return task_group_id in self.task_group_dict
+
+    @property
+    def task_group_dict(self):
+        return {k: v for k, v in 
self._task_group.get_task_group_dict().items() if k is not None}

Review Comment:
   I’m kind of wondering if we really need to exclude `None` (the root group) 
here. It doesn’t really change how everything behaves.
   
   Also this is likely better to be a `cached_property`.



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