Skip to content

Commit 61836f1

Browse files
author
gibsondan
authored
remove asset_graph argument from AssetGraphSubset.get_asset_subset (#31197)
Summary: Goal is to move as much as possible of the asset-graph-dependant logic here to AssetGraphView and treat AssetGraphSubset as a relatively dumb data class. Test Plan: BK
1 parent 8f9a251 commit 61836f1

4 files changed

Lines changed: 33 additions & 41 deletions

File tree

python_modules/dagster/dagster/_core/asset_graph_view/asset_graph_view.py

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,12 @@ def get_entity_subset_from_asset_graph_subset(
238238
self.asset_graph.has(key), f"Asset graph does not contain {key.to_user_string()}"
239239
)
240240

241-
serializable_subset = self._with_current_partitions_def(
242-
asset_graph_subset.get_asset_subset(key, self.asset_graph)
243-
)
241+
serializable_subset = asset_graph_subset.get_asset_subset(key)
242+
243+
if not serializable_subset:
244+
return self.get_empty_subset(key=key)
245+
246+
serializable_subset = self._with_current_partitions_def(serializable_subset)
244247

245248
return EntitySubset(
246249
self, key=key, value=_ValidatedEntitySubsetValue(serializable_subset.value)
@@ -626,12 +629,8 @@ async def _compute_run_in_progress_asset_subset(self, key: AssetKey) -> EntitySu
626629
async def _compute_backfill_in_progress_asset_subset(
627630
self, key: AssetKey
628631
) -> EntitySubset[AssetKey]:
629-
value = (
630-
self._queryer.get_active_backfill_in_progress_asset_graph_subset()
631-
.get_asset_subset(asset_key=key, asset_graph=self.asset_graph)
632-
.value
633-
)
634-
return EntitySubset(self, key=key, value=_ValidatedEntitySubsetValue(value))
632+
asset_graph_subset = self._queryer.get_active_backfill_in_progress_asset_graph_subset()
633+
return self.get_entity_subset_from_asset_graph_subset(asset_graph_subset, key)
635634

636635
async def _compute_execution_failed_unpartitioned(self, key: AssetKey) -> bool:
637636
from dagster._core.event_api import AssetRecordsFilter

python_modules/dagster/dagster/_core/definitions/assets/graph/asset_graph_subset.py

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -67,35 +67,15 @@ def num_partitions_and_non_partitioned_assets(self) -> int:
6767
def is_empty(self) -> bool:
6868
return len(self.asset_keys) == 0
6969

70-
def _get_serializable_entity_subset(
71-
self, asset_key: AssetKey
72-
) -> SerializableEntitySubset[AssetKey]:
70+
def get_asset_subset(self, asset_key: AssetKey) -> Optional[SerializableEntitySubset[AssetKey]]:
7371
if asset_key in self.non_partitioned_asset_keys:
7472
return SerializableEntitySubset(key=asset_key, value=True)
7573
elif asset_key in self.partitions_subsets_by_asset_key:
7674
return SerializableEntitySubset(
7775
key=asset_key, value=self.partitions_subsets_by_asset_key[asset_key]
7876
)
7977
else:
80-
check.failed(f"Asset {asset_key} must be part of the AssetGraphSubset")
81-
82-
def get_asset_subset(
83-
self, asset_key: AssetKey, asset_graph: BaseAssetGraph
84-
) -> SerializableEntitySubset[AssetKey]:
85-
"""Returns an AssetSubset representing the subset of a specific asset that this
86-
AssetGraphSubset contains.
87-
"""
88-
if (
89-
asset_key in self.non_partitioned_asset_keys
90-
or asset_key in self.partitions_subsets_by_asset_key
91-
):
92-
return self._get_serializable_entity_subset(asset_key)
93-
else:
94-
partitions_def = asset_graph.get(asset_key).partitions_def
95-
return SerializableEntitySubset(
96-
key=asset_key,
97-
value=partitions_def.empty_subset() if partitions_def else False,
98-
)
78+
return None
9979

10080
def get_partitions_subset(
10181
self, asset_key: AssetKey, asset_graph: Optional[BaseAssetGraph] = None
@@ -127,7 +107,7 @@ def iterate_asset_subsets(self) -> Iterable[SerializableEntitySubset[AssetKey]]:
127107
AssetGraphSubset contains.
128108
"""
129109
for asset_key in self.asset_keys:
130-
yield self._get_serializable_entity_subset(asset_key)
110+
yield check.not_none(self.get_asset_subset(asset_key))
131111

132112
def __contains__(self, asset: Union[AssetKey, AssetKeyPartitionKey]) -> bool: # pyright: ignore[reportIncompatibleMethodOverride]
133113
"""If asset is an AssetKeyPartitionKey, check if the given AssetKeyPartitionKey is in the

python_modules/dagster/dagster/_core/definitions/auto_materialize_rule_impls.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,14 +1056,19 @@ def evaluate_for_asset(self, context: "AutomationContext") -> "AutomationResult"
10561056
AutomationResult,
10571057
)
10581058

1059+
# this backfilling subset is aware of the current partitions definitions, and so will
1060+
# be valid
1061+
asset_subset = (
1062+
context.legacy_context.instance_queryer.get_active_backfill_target_asset_graph_subset().get_asset_subset(
1063+
context.legacy_context.asset_key
1064+
)
1065+
or context.asset_graph_view.get_empty_subset(
1066+
key=context.legacy_context.asset_key
1067+
).convert_to_serializable_subset()
1068+
)
1069+
10591070
backfilling_subset = ValidAssetSubset.coerce_from_subset(
1060-
# this backfilling subset is aware of the current partitions definitions, and so will
1061-
# be valid
1062-
(
1063-
context.legacy_context.instance_queryer.get_active_backfill_target_asset_graph_subset()
1064-
).get_asset_subset(
1065-
context.legacy_context.asset_key, context.legacy_context.asset_graph
1066-
),
1071+
asset_subset,
10671072
context.legacy_context.partitions_def,
10681073
)
10691074

python_modules/dagster/dagster/_core/execution/asset_backfill.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1961,8 +1961,16 @@ def _get_cant_run_because_of_parent_reason(
19611961
if parent_node.partitions_def != candidate_node.partitions_def:
19621962
return f"parent {parent_node.key.to_user_string()} and {candidate_node.key.to_user_string()} have different partitions definitions so they cannot be materialized in the same run. {candidate_node.key.to_user_string()} can be materialized once {parent_node.key.to_user_string()} is materialized."
19631963

1964-
parent_target_subset = target_subset.get_asset_subset(parent_asset_key, asset_graph)
1965-
candidate_target_subset = target_subset.get_asset_subset(candidate_asset_key, asset_graph)
1964+
parent_target_subset = (
1965+
target_subset.get_asset_subset(parent_asset_key)
1966+
or asset_graph_view.get_empty_subset(key=parent_asset_key).convert_to_serializable_subset()
1967+
)
1968+
candidate_target_subset = (
1969+
target_subset.get_asset_subset(candidate_asset_key)
1970+
or asset_graph_view.get_empty_subset(
1971+
key=candidate_asset_key
1972+
).convert_to_serializable_subset()
1973+
)
19661974

19671975
num_parent_partitions_being_requested_this_tick = parent_being_requested_this_tick_subset.size
19681976

0 commit comments

Comments
 (0)