dstandish commented on code in PR #37101:
URL: https://github.com/apache/airflow/pull/37101#discussion_r1499742378


##########
airflow/jobs/scheduler_job_runner.py:
##########


Review Comment:
   the changes to this file were in a previous version of my foundation PR, 
checking one at a time for exists.  i ultimately reverted that change before 
merging because i wasn't sure we needed it. i would keep that out of this PR 
cus it's unrelated.  we can consider it independently.  so i would revert the 
changes to th is file.



##########
airflow/datasets/__init__.py:
##########
@@ -106,8 +112,90 @@ class DatasetAny(_DatasetBooleanCondition):
 
     agg_func = any
 
+    def __init__(self, *objects: Dataset | DatasetAny | DatasetAll) -> None:
+        """Initialize with one or more Dataset, DatasetAny, or DatasetAll 
instances."""
+        super().__init__(*objects)
+
+    def __or__(self, other):
+        if isinstance(other, (Dataset, DatasetAny, DatasetAll)):
+            return DatasetAny(*self.objects, other)
+        return NotImplemented
+
+    def __and__(self, other):
+        if isinstance(other, (Dataset, DatasetAny, DatasetAll)):
+            return DatasetAll(self, other)
+        return NotImplemented
+
+    def __repr__(self) -> str:
+        return f"DatasetAny({', '.join(map(str, self.objects))})"
+
 
 class DatasetAll(_DatasetBooleanCondition):
     """Use to combine datasets schedule references in an "or" relationship."""
 
     agg_func = all
+
+    def __init__(self, *objects: Dataset | DatasetAny | DatasetAll):
+        """Initialize with one or more Dataset, DatasetAny, or DatasetAll 
instances."""
+        super().__init__(*objects)
+
+    def __or__(self, other):
+        if isinstance(other, (Dataset, DatasetAny, DatasetAll)):
+            return DatasetAny(self, other)
+        return NotImplemented
+
+    def __and__(self, other):
+        if isinstance(other, (Dataset, DatasetAny, DatasetAll)):
+            return DatasetAll(*self.objects, other)
+        return NotImplemented
+
+    def __repr__(self) -> str:
+        return f"DatasetAll({', '.join(map(str, self.objects))})"
+
+
+class DatasetsExpression:
+    """
+    Represents a node in an expression tree for dataset conditions.
+
+    :param value: The value of the node, which can be a 'Dataset', '&', or '|'.
+    :param left: The left child node.
+    :param right: The right child node.
+    """
+
+    def __init__(self, value, left=None, right=None) -> None:
+        self.value = value  # value can be 'Dataset', '&', or '|'
+        self.left = left
+        self.right = right
+
+    def __or__(self, other: Dataset | DatasetsExpression) -> 
DatasetsExpression:
+        return DatasetsExpression("|", self, other)

Review Comment:
   it does feel a little bit awkward to use the strings `|` and `&`  as values 
to signal how to handle.  maybe there's a better way to accomplish using 
subclasses?



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