Make @graph_asset support explicit key #16751
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
schrockn
force-pushed
the
graph-asset
branch
from
06e0268 to
176ed79
Compare
schrockn
requested review from
johannkm,
sryza and
alangenfeld
and removed request for
erinkcochran87
schrockn
force-pushed
the
graph-asset
branch
from
176ed79 to
091d5f5
Compare
|
Deploy preview for dagit-storybook ready! ✅ Preview Built with commit 091d5f5. |
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 091d5f5. |
|
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-fx2nwxyqz-elementl.vercel.app Direct link to changed pages: |
## Summary & Motivation Takes the duplicated logic added in #16751 and consolidates it to a single helper function. There is a surprising amount of incidental complexity in this logic, so stuffing it into a common helper is a win. I did this as a separate PR so that we can revert it independently in case this introduces a bug in `@asset`. ## How I Tested These Changes BK
Summary & Motivation
graph_asset's parameters are out-of-sync withasset's on a structural level, and we do not have the discipline or the tests to keep them in sync. This PR adds an explicitkeywhich is nice for users and would have made a recent PR of @johannkm's nicer, who had to do some contortions in #16612 aroundkey_prefix.This diff also adds documentation for
keyon@asset.An upstack PR will consolidate the logic in
@assetand@graph_assetaround this stuff into a single helper function, since it is pretty gross, tricky code.How I Tested These Changes