You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/14 19:13:05 UTC

[GitHub] [airflow] ashb commented on a change in pull request #20297: Rename TaskMixin to DependencyMixin

ashb commented on a change in pull request #20297:
URL: https://github.com/apache/airflow/pull/20297#discussion_r768968186



##########
File path: airflow/models/taskmixin.py
##########
@@ -15,61 +15,78 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import warnings
 from abc import abstractmethod
 from typing import Sequence, Union
 
+DependencyMixinOrList = Union["DependencyMixin", Sequence["DependencyMixin"]]
 
-class TaskMixin:
-    """
-    Mixing implementing common chain methods like >> and <<.
 
-    In the following functions we use:
-    Task = Union[BaseOperator, XComArg]
-    No type annotations due to cyclic imports.
-    """
+class DependencyMixin:
+    """Mixing implementing common dependency setting methods methods like >> and <<."""
 
     @property
-    def roots(self):
-        """Should return list of root operator List[BaseOperator]"""
+    def roots(self) -> Sequence["DependencyMixin"]:
+        """
+        List of root nodes -- ones with no upstream dependencies.
+
+        a.k.a. the "start" of this sub-graph
+        """
         raise NotImplementedError()
 
     @property
-    def leaves(self):
-        """Should return list of leaf operator List[BaseOperator]"""
+    def leaves(self) -> Sequence["DependencyMixin"]:
+        """
+        List of leaf nodes -- ones with only upstream dependencies.
+
+        a.k.a. the "end" of this sub-graph
+        """
         raise NotImplementedError()
 
     @abstractmethod
-    def set_upstream(self, other: Union["TaskMixin", Sequence["TaskMixin"]]):
+    def set_upstream(self, other: DependencyMixinOrList):

Review comment:
       I'm not 100% sold on this name, but it felt very verbose the number of places we had `Union["TaskMixin", Sequence["TaskMixin"]]` used.
   
   Is this actually any easier to understand, or is the indirection just hiding this and making it harder to grok?




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

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