-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
order dependencies when subsetting pipeline #4851
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/elementl/dagster/EovtTSi8rmxCHokKQ4zv6nAQjAkV |
f4558c8
to
6d2f4c6
Compare
@@ -627,7 +632,7 @@ def _get_pipeline_subset_def( | |||
deps[_dep_key_of(solid)][input_handle.input_def.name] = DependencyDefinition( | |||
solid=output_handle.solid.name, output=output_handle.output_def.name | |||
) | |||
if graph.dependency_structure.has_dynamic_fan_in_dep(input_handle): | |||
elif graph.dependency_structure.has_dynamic_fan_in_dep(input_handle): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are equivalent because they're mutually exclusive conditions, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya, an input can only have one type of dep
solids = list(map(graph.solid_named, solids_to_execute)) | ||
# go in topo order to ensure deps dict is ordered | ||
solids = list( | ||
filter(lambda solid: solid.name in solids_to_execute, graph.solids_in_topological_order) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this causing a grpc error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was throwing a check exception in the user code when building the subset pipeline since the dynamic output resolution alogrithm expects to process the dependencies in order
resolves #4848 ### Test Plan added repro test case base on issue
resolves #4848
Test Plan
added repro test case base on issue