Subset multi_graph_asset with checks #20115
Conversation
|
Current dependencies on/for this PR: This stack of pull requests is managed by Graphite. |
johannkm
force-pushed
the
johann/02-26-Subset_multi_graph_asset_with_checks
branch
from
f1940b6
to
8710079
Compare
johannkm
force-pushed
the
johann/02-26-Subset_multi_graph_asset_with_checks
branch
from
8710079
to
5d76c63
Compare
johannkm
force-pushed
the
johann/02-26-Subset_multi_graph_asset_with_checks
branch
from
5d76c63
to
368e062
Compare
|
test failures are unrelated |
| ] = [] # first element in list is node that outputs asset | ||
|
|
||
| dep_node_outputs_by_asset_key[asset_key] = [] | ||
| dep_node_outputs_by_asset_or_check_key[asset_or_check_key] = [] |
There was a problem hiding this comment.
Looks like none of the callsites actually require dep_node_outputs_by_asset_or_check_key to contain check information, can we instead exclusively change dep_nodes_by_asset_or_check_key?
(I think we only need to add check information to dep_node_outputs_by_asset_or_check_key if we needed to support calling context.selected_output_names from within a check op)
There was a problem hiding this comment.
Without it we hit Could not find output mapping asset_one_check1 when resolving the job
|
|
||
|
|
||
| def test_graph_asset_subset_no_checks(): | ||
| result = materialize( |
There was a problem hiding this comment.
My understanding is that it's currently not possible to execute checks alone without executing the corresponding asset, is that correct?
If not, this would be a good case to test.
There was a problem hiding this comment.
https://github.com/dagster-io/dagster/pull/20115/files#diff-66c934e188c377f74363d3f1764d1a8de0a96001b69edd54e54821d53549ffe2R1542 prevents just checks from being launched from host processes. Ideally we'd unblock this but I think it can wait, I'll file a tracking issue
johannkm
force-pushed
the
johann/02-26-Subset_multi_graph_asset_with_checks
branch
from
368e062
to
cacdb30
Compare
|
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit cacdb30. |
This reverts commit cacdb30.
johannkm
force-pushed
the
johann/02-26-Subset_multi_graph_asset_with_checks
branch
from
8e68099
to
7045db6
Compare
Closes #19915 Remove the block against subsetting `@graph_multi_asset`s with checks, and include check outputs in the `op_selection` calculation. The result is that subsetting with checks behaves like subsetting with just assets- we filter the graph down to the required nodes. Test plan: unit tests, manually testing in toys
Closes #19915 Remove the block against subsetting `@graph_multi_asset`s with checks, and include check outputs in the `op_selection` calculation. The result is that subsetting with checks behaves like subsetting with just assets- we filter the graph down to the required nodes. Test plan: unit tests, manually testing in toys
Closes #19915
Remove the block against subsetting
@graph_multi_assets with checks, and include check outputs in theop_selectioncalculation. The result is that subsetting with checks behaves like subsetting with just assets- we filter the graph down to the required nodes.Test plan: unit tests, manually testing in toys