[GitHub] [airflow] casassg commented on a change in pull request #10153: [AIP-34] Alternative proposal: Add TaskGroup as a DAG construction helper and UI concept with example

[GitBox <gi...@apache.org>]

casassg commented on a change in pull request #10153:
URL: https://github.com/apache/airflow/pull/10153#discussion_r480247681



##########
File path: airflow/models/baseoperator.py
##########
@@ -382,7 +389,16 @@ def __init__(
                 stacklevel=3
             )
         validate_key(task_id)
-        self.task_id = task_id
+        self.label = task_id
+
+        # Prefix task_id with group_id
+        task_group = task_group or TaskGroupContext.get_current_task_group(dag)
+        if task_group:
+            self.task_id = f"{task_group.group_id}.{self.label}" if task_group.group_id else self.label

Review comment:
       > Do not prefix task_id with group_id
   
   This is a downgrade from current SubDagOperator. You can have duplicated task_id across subdags in the same dag at the moment. I would suggest actually prefixing.
   
   Also this change is a divergence with what's voted and proposed in https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-34+TaskGroup%3A+A+UI+task+grouping+concept+as+an+alternative+to+SubDagOperator. I would either bring this up for discussion on the mailing list and wait for a lazy consensus, or refactor AIP and re-submit for vote. 




----------------------------------------------------------------
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:
users@infra.apache.org