Support description in AssetSpec on multi_asset. Leave comment explaining muddled nature of this codepath #16755
Conversation
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
schrockn
changed the title
schrockn
force-pushed
the
fix-descriptions-take-two
branch
from
ee706e5 to
fe2c135
Compare
schrockn
force-pushed
the
fix-descriptions-take-two
branch
from
fe2c135 to
e282a16
Compare
|
Deploy preview for dagster-university ready! ✅ Preview Built with commit e282a16. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit e282a16. |
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit e282a16. |
add explantory comment better comment comments cp
schrockn
force-pushed
the
fix-descriptions-take-two
branch
from
e282a16 to
c4586aa
Compare
|
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-8sqbf7nlw-elementl.vercel.app Direct link to changed pages: |
…ning muddled nature of this codepath (#16755) ## Summary & Motivation This was super confusing as by inspection it appeared that `multi_asset` did not support descriptions. However there are actually three sources of truth for descriptions, one in `OutputDefinition`, one in `NodeDefinition`, and one in the `descriptions_by_key`. This last is best thought of as an "override." This goes in the annals of "un-composability and ambiguously immutable Ls". There should be a canonical source of truth of descriptions once we are deep within the depths of `AssetsDefinition`. If the dictionary is needed for perf reasons (which I doubt), we can build a memoized cache of descriptions derived from the immutable data. When the immutable data is passed to a new copy of `AssetsDefinition` when we need it, we can recompute the memoized dict. ## How I Tested These Changes BK
Summary & Motivation
This was super confusing as by inspection it appeared that
multi_assetdid not support descriptions. However there are actually three sources of truth for descriptions, one inOutputDefinition, one inNodeDefinition, and one in thedescriptions_by_key. This last is best thought of as an "override."This goes in the annals of "un-composability and ambiguously immutable Ls". There should be a canonical source of truth of descriptions once we are deep within the depths of
AssetsDefinition. If the dictionary is needed for perf reasons (which I doubt), we can build a memoized cache of descriptions derived from the immutable data. When the immutable data is passed to a new copy ofAssetsDefinitionwhen we need it, we can recompute the memoized dict.How I Tested These Changes
BK