pankajkoti commented on code in PR #39604:
URL: https://github.com/apache/airflow/pull/39604#discussion_r1603872904


##########
airflow/cli/commands/task_command.py:
##########
@@ -177,7 +176,7 @@ def _get_ti_db_access(
 
     if not exec_date_or_run_id and not create_if_necessary:
         raise ValueError("Must provide `exec_date_or_run_id` if not 
`create_if_necessary`.")
-    if needs_expansion(task):
+    if task.get_needs_expansion():
         if map_index < 0:
             raise RuntimeError("No map_index passed to mapped task")

Review Comment:
   ```suggestion
       if task.get_needs_expansion() and map_index < 0:
           raise RuntimeError("No map_index passed to mapped task")
   ```



##########
airflow/models/abstractoperator.py:
##########
@@ -395,6 +395,19 @@ def get_closest_mapped_task_group(self) -> MappedTaskGroup 
| None:
         """
         return next(self.iter_mapped_task_groups(), None)
 
+    def get_needs_expansion(self) -> bool:

Review Comment:
   Could it be better if we call this method `should_expand` / 
`needs_expansion` / `is_expansion_needed`?
   
   Then `task.should_expand()` or `task.needs_expansion()` or 
`task.is_expansion_needed()` appears more explanatory than 
`task.get_needs_expansion()`. That's however just my thought when I first read 
that :) 



-- 
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-unsubscr...@airflow.apache.org

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

Reply via email to