dagster
dagster copied to clipboard
[spec-ify-assets-def] replace dicts inside AssetsDefinition with dict of AssetSpecs
Summary & Motivation
This PR is the star of the "[spec-ify-assets-def]" stack.
It replaces all of these properties on AssetsDefinition
:
_asset_deps: Mapping[AssetKey, AbstractSet[AssetKey]]
_group_names_by_key: Mapping[AssetKey, str]
_metadata_by_key: Mapping[AssetKey, ArbitraryMetadataMapping]
_tags_by_key: Mapping[AssetKey, Mapping[str, str]]
_freshness_policies_by_key: Mapping[AssetKey, FreshnessPolicy]
_auto_materialize_policies_by_key: Mapping[AssetKey, AutoMaterializePolicy]
_code_versions_by_key: Mapping[AssetKey, Optional[str]]
_descriptions_by_key: Mapping[AssetKey, str]
_owners_by_key: Mapping[AssetKey, Sequence[str]]
with just:
_specs_by_key: Mapping[AssetKey, AssetSpec]
It should have no effect on behavior or public APIs.
A couple things to think about here:
- Perf. Every time we copy an
AssetsDefinition
, we need to go back and forth between the two representations. I have followup PRs that make this no longer the case, but that are more invasive. -
AssetSpec
vs.AssetDefinition
vs.AssetNode
. At times, we've discussed the possibility thatAssetSpec
should not be the internal object of record that we use to describe assets. This change does not lock us intoAssetSpec
being the internal object of record. By consolidating all these dicts into one place, this change should make it easier to swap it out for something else in the future if we choose to do so.