Remove description property, make names more descriptive

dagster-io · Oct 10, 2024 · 1357a85 · 1357a85
1 parent 5459582
commit 1357a85
Show file tree

Hide file tree

Showing 9 changed files with 44 additions and 99 deletions.
diff --git a/..._modules/dagster/dagster/_core/definitions/declarative_automation/automation_condition.py b/..._modules/dagster/dagster/_core/definitions/declarative_automation/automation_condition.py
@@ -124,7 +124,7 @@ def get_snapshot(
 
     def get_node_unique_id(self, *, parent_unique_id: Optional[str], index: Optional[int]) -> str:
         """Returns a unique identifier for this condition within the broader condition tree."""
-        parts = [str(parent_unique_id), str(index), self.__class__.__name__, self.description]
+        parts = [str(parent_unique_id), str(index), self.name]
         return non_secure_md5_hash_str("".join(parts).encode())
 
     def get_unique_id(
@@ -458,9 +458,7 @@ def cron_tick_passed(
             CronTickPassedCondition,
         )
 
-        return CronTickPassedCondition(
-            cron_schedule=cron_schedule, cron_timezone=cron_timezone
-        ).with_label(f"cron_tick_passed({cron_schedule}, {cron_timezone})")
+        return CronTickPassedCondition(cron_schedule=cron_schedule, cron_timezone=cron_timezone)
 
     @experimental
     @staticmethod

diff --git a/...gster/_core/definitions/declarative_automation/operands/code_version_changed_condition.py b/...gster/_core/definitions/declarative_automation/operands/code_version_changed_condition.py
@@ -11,10 +11,6 @@
 @whitelist_for_serdes
 @record
 class CodeVersionChangedCondition(BuiltinAutomationCondition[AssetKey]):
-    @property
-    def description(self) -> str:
-        return "Asset code version changed since previous tick"
-
     @property
     def name(self) -> str:
         return "code_version_changed"

diff --git a/...les/dagster/dagster/_core/definitions/declarative_automation/operands/slice_conditions.py b/...les/dagster/dagster/_core/definitions/declarative_automation/operands/slice_conditions.py
@@ -20,10 +20,6 @@
 class InitialEvaluationCondition(BuiltinAutomationCondition):
     """Condition to determine if this is the initial evaluation of a given AutomationCondition."""
 
-    @property
-    def description(self) -> str:
-        return "Initial evaluation"
-
     @property
     def name(self) -> str:
         return "initial_evaluation"
@@ -63,10 +59,6 @@ def evaluate(self, context: AutomationContext[T_EntityKey]) -> AutomationResult[
 @whitelist_for_serdes
 @record
 class MissingAutomationCondition(SubsetAutomationCondition):
-    @property
-    def description(self) -> str:
-        return "Missing"
-
     @property
     def name(self) -> str:
         return "missing"
@@ -80,10 +72,6 @@ def compute_subset(self, context: AutomationContext) -> EntitySubset:
 @whitelist_for_serdes
 @record
 class InProgressAutomationCondition(SubsetAutomationCondition):
-    @property
-    def description(self) -> str:
-        return "Part of an in-progress run"
-
     @property
     def name(self) -> str:
         return "in_progress"
@@ -95,10 +83,6 @@ def compute_subset(self, context: AutomationContext) -> EntitySubset:
 @whitelist_for_serdes(storage_name="FailedAutomationCondition")
 @record
 class ExecutionFailedAutomationCondition(SubsetAutomationCondition):
-    @property
-    def description(self) -> str:
-        return "Latest execution failed"
-
     @property
     def name(self) -> str:
         return "execution_failed"
@@ -144,10 +128,6 @@ def compute_subset(self, context: AutomationContext) -> EntitySubset:
 @whitelist_for_serdes
 @record
 class NewlyRequestedCondition(SubsetAutomationCondition):
-    @property
-    def description(self) -> str:
-        return "Was requested on the previous tick"
-
     @property
     def name(self) -> str:
         return "newly_requested"
@@ -159,10 +139,6 @@ def compute_subset(self, context: AutomationContext) -> EntitySubset:
 @whitelist_for_serdes
 @record
 class NewlyUpdatedCondition(SubsetAutomationCondition):
-    @property
-    def description(self) -> str:
-        return "Updated since previous tick"
-
     @property
     def name(self) -> str:
         return "newly_updated"
@@ -188,13 +164,9 @@ class CronTickPassedCondition(SubsetAutomationCondition):
     cron_schedule: str
     cron_timezone: str
 
-    @property
-    def description(self) -> str:
-        return f"New tick of {self.cron_schedule} ({self.cron_timezone})"
-
     @property
     def name(self) -> str:
-        return "cron_tick_passed"
+        return f"cron_tick_passed(cron_schedule={self.cron_schedule}, cron_timezone={self.cron_timezone})"
 
     def _get_previous_cron_tick(self, effective_dt: datetime.datetime) -> datetime.datetime:
         previous_ticks = reverse_cron_string_iterator(
@@ -250,7 +222,10 @@ def description(self) -> str:
 
     @property
     def name(self) -> str:
-        return "in_latest_time_window"
+        name = "in_latest_time_window"
+        if self.serializable_lookback_timedelta:
+            name += f"(lookback_timedelta={self.lookback_timedelta})"
+        return name
 
     def compute_subset(self, context: AutomationContext) -> EntitySubset:
         return context.asset_graph_view.compute_latest_time_window_subset(

diff --git a/.../_core/definitions/declarative_automation/operators/any_downstream_conditions_operator.py b/.../_core/definitions/declarative_automation/operators/any_downstream_conditions_operator.py
@@ -22,8 +22,8 @@ class DownstreamConditionWrapperCondition(BuiltinAutomationCondition[AssetKey]):
     operand: AutomationCondition
 
     @property
-    def description(self) -> str:
-        return ",".join(key.to_user_string() for key in self.downstream_keys)
+    def name(self) -> str:
+        return ", ".join(key.to_user_string() for key in self.downstream_keys)
 
     @property
     def children(self) -> Sequence[AutomationCondition]:

diff --git a/...les/dagster/dagster/_core/definitions/declarative_automation/operators/check_operators.py b/...les/dagster/dagster/_core/definitions/declarative_automation/operators/check_operators.py
@@ -24,16 +24,14 @@ class ChecksAutomationCondition(BuiltinAutomationCondition[AssetKey]):
 
     @property
     @abstractmethod
-    def base_description(self) -> str: ...
+    def base_name(self) -> str: ...
 
     @property
-    def description(self) -> str:
-        description = f"{self.base_description} "
-        if self.blocking_only:
-            description += "blocking checks"
+    def name(self) -> str:
+        name = self.base_name
         if self.blocking_only:
-            description += "checks"
-        return description
+            name += "(blocking_only=True)"
+        return name
 
     @property
     def requires_cursor(self) -> bool:
@@ -53,11 +51,7 @@ def _get_check_keys(
 @record
 class AnyChecksCondition(ChecksAutomationCondition):
     @property
-    def base_description(self) -> str:
-        return "Any"
-
-    @property
-    def name(self) -> str:
+    def base_name(self) -> str:
         return "ANY_CHECKS_MATCH"
 
     def evaluate(self, context: AutomationContext[AssetKey]) -> AutomationResult[AssetKey]:
@@ -86,11 +80,7 @@ def evaluate(self, context: AutomationContext[AssetKey]) -> AutomationResult[Ass
 @record
 class AllChecksCondition(ChecksAutomationCondition):
     @property
-    def base_description(self) -> str:
-        return "All"
-
-    @property
-    def name(self) -> str:
+    def base_name(self) -> str:
         return "ALL_CHECKS_MATCH"
 
     def evaluate(self, context: AutomationContext[AssetKey]) -> AutomationResult[AssetKey]:

diff --git a/...dules/dagster/dagster/_core/definitions/declarative_automation/operators/dep_operators.py b/...dules/dagster/dagster/_core/definitions/declarative_automation/operators/dep_operators.py
@@ -52,16 +52,20 @@ class DepsAutomationCondition(BuiltinAutomationCondition[T_EntityKey]):
 
     @property
     @abstractmethod
-    def base_description(self) -> str: ...
+    def base_name(self) -> str: ...
 
     @property
-    def description(self) -> str:
-        description = f"{self.base_description} deps"
+    def name(self) -> str:
+        name = self.base_name
+        props = []
         if self.allow_selection is not None:
-            description += f" within selection {self.allow_selection}"
+            props.append("allow_selection={self.allow_selection}")
         if self.ignore_selection is not None:
-            description += f" except for {self.ignore_selection}"
-        return description
+            props.append("ignore_selection={self.ignore_selection}")
+
+        if props:
+            name += f"({','.join(props)})"
+        return name
 
     @property
     def requires_cursor(self) -> bool:
@@ -105,11 +109,7 @@ def _get_dep_keys(
 @whitelist_for_serdes
 class AnyDepsCondition(DepsAutomationCondition[T_EntityKey]):
     @property
-    def base_description(self) -> str:
-        return "Any"
-
-    @property
-    def name(self) -> str:
+    def base_name(self) -> str:
         return "ANY_DEPS_MATCH"
 
     def evaluate(self, context: AutomationContext[T_EntityKey]) -> AutomationResult[T_EntityKey]:
@@ -135,11 +135,7 @@ def evaluate(self, context: AutomationContext[T_EntityKey]) -> AutomationResult[
 @whitelist_for_serdes
 class AllDepsCondition(DepsAutomationCondition[T_EntityKey]):
     @property
-    def base_description(self) -> str:
-        return "All"
-
-    @property
-    def name(self) -> str:
+    def base_name(self) -> str:
         return "ALL_DEPS_MATCH"
 
     def evaluate(self, context: AutomationContext[T_EntityKey]) -> AutomationResult[T_EntityKey]:

diff --git a/...dagster/dagster/_core/definitions/declarative_automation/operators/newly_true_operator.py b/...dagster/dagster/_core/definitions/declarative_automation/operators/newly_true_operator.py
@@ -18,10 +18,6 @@
 class NewlyTrueCondition(BuiltinAutomationCondition[T_EntityKey]):
     operand: AutomationCondition[T_EntityKey]
 
-    @property
-    def description(self) -> str:
-        return "Condition newly became true."
-
     @property
     def name(self) -> str:
         return "NEWLY_TRUE"

diff --git a/...ules/dagster/dagster/_core/definitions/declarative_automation/operators/since_operator.py b/...ules/dagster/dagster/_core/definitions/declarative_automation/operators/since_operator.py
@@ -17,12 +17,6 @@ class SinceCondition(BuiltinAutomationCondition[T_EntityKey]):
     trigger_condition: AutomationCondition[T_EntityKey]
     reset_condition: AutomationCondition[T_EntityKey]
 
-    @property
-    def description(self) -> str:
-        return (
-            "Trigger condition has become true since the last time the reset condition became true."
-        )
-
     @property
     def name(self) -> str:
         return "SINCE"

diff --git a/...rative_automation_tests/automation_condition_tests/fundamentals/test_result_value_hash.py b/...rative_automation_tests/automation_condition_tests/fundamentals/test_result_value_hash.py
@@ -31,24 +31,24 @@
     [
         # cron condition returns a unique value hash if parents change, if schedule changes, if the
         # partitions def changes, or if an asset is materialized
-        ("ad228f0044da1efba407e794c845e858", SC.on_cron("0 * * * *"), one_parent, False),
-        ("f34de3cd3e1ab283a95a892192437076", SC.on_cron("0 * * * *"), one_parent, True),
-        ("d9533b4eb0aad1798d5da85520b9852c", SC.on_cron("0 0 * * *"), one_parent, False),
-        ("8a233d38e569faba1470b0717c28fbee", SC.on_cron("0 * * * *"), one_parent_daily, False),
-        ("e8fa53c550e99edc1346a1f80979cddd", SC.on_cron("0 * * * *"), two_parents, False),
-        ("5c58fb8fc117d69b32e734c45af219ea", SC.on_cron("0 * * * *"), two_parents_daily, False),
+        ("93831bb3ab9c6ef4b10a7f823ce5cc1f", SC.on_cron("0 * * * *"), one_parent, False),
+        ("97ea4d62fcef2f6a2ecc99a95d5c1769", SC.on_cron("0 * * * *"), one_parent, True),
+        ("504192a87594854d3964bb03e2092394", SC.on_cron("0 0 * * *"), one_parent, False),
+        ("ccbd282bf8e6d711c2bb0e01ebb16728", SC.on_cron("0 * * * *"), one_parent_daily, False),
+        ("dd74c7cfe19d869931ea4aad9ee10127", SC.on_cron("0 * * * *"), two_parents, False),
+        ("861f8e40d4624d49c4ebdd034c8e1e84", SC.on_cron("0 * * * *"), two_parents_daily, False),
         # same as above
-        ("678e0a2be6bba89bd2d37fb432d8fb51", SC.eager(), one_parent, False),
-        ("72bd068363441e02d67b3407fe3e9cae", SC.eager(), one_parent, True),
-        ("4f0e2e38131ae91b1b9408e3cd549dd0", SC.eager(), one_parent_daily, False),
-        ("00ecedd77a8d887940856950c556c7d1", SC.eager(), two_parents, False),
-        ("6ad1fd331c63c75e17572ec60b9c27b5", SC.eager(), two_parents_daily, False),
+        ("b5cb0d7a1c627bd2c9e7c6da3313ab71", SC.eager(), one_parent, False),
+        ("7802a65024d04bbe44a3e0e541c0a577", SC.eager(), one_parent, True),
+        ("5d9c70da7ecca9e40f32c1ad99956b5d", SC.eager(), one_parent_daily, False),
+        ("904bac575906542d28b9e069129dad37", SC.eager(), two_parents, False),
+        ("3ef1d373a2b38752ad8e23fe9c053d9f", SC.eager(), two_parents_daily, False),
         # missing condition is invariant to changes other than partitions def changes
-        ("5c24ffc21af9983a4917b91290de8f5d", SC.missing(), one_parent, False),
-        ("5c24ffc21af9983a4917b91290de8f5d", SC.missing(), one_parent, True),
-        ("5c24ffc21af9983a4917b91290de8f5d", SC.missing(), two_parents, False),
-        ("c722c1abf97c5f5fe13b2f6fc00af739", SC.missing(), two_parents_daily, False),
-        ("c722c1abf97c5f5fe13b2f6fc00af739", SC.missing(), one_parent_daily, False),
+        ("6d7809c4949e3d812d7eddfb1b60d529", SC.missing(), one_parent, False),
+        ("6d7809c4949e3d812d7eddfb1b60d529", SC.missing(), one_parent, True),
+        ("6d7809c4949e3d812d7eddfb1b60d529", SC.missing(), two_parents, False),
+        ("7f852ab7408c67e0830530d025505a37", SC.missing(), two_parents_daily, False),
+        ("7f852ab7408c67e0830530d025505a37", SC.missing(), one_parent_daily, False),
     ],
 )
 def test_value_hash(