Page MenuHomePhabricator

[dagster] fix composition bug
ClosedPublic

Authored by alangenfeld on Tue, Aug 20, 6:39 PM.

Details

Reviewers
schrockn
Group Reviewers
Restricted Project
Commits
R1:32f8b7f63ac5: [dagster] fix composition bug
Summary

This terminal_step_output jonx did not get handled correctly for the entirety of composites existence. We never tested the case where you have fully parallel subdags within a composite which is where it breaks down.

To handle correctly, we build up a dict of compute steps as we recurse and then use the mapping path to resolve through any output mappings to the actual compute step it comes from.

Fixes https://github.com/dagster-io/dagster/issues/1674

Test Plan

newly added test case

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

alangenfeld updated this revision to Diff 3858.Tue, Aug 20, 6:39 PM
alangenfeld created this revision.

cleamn

alangenfeld edited the summary of this revision. (Show Details)Tue, Aug 20, 6:41 PM
Harbormaster completed remote builds in B3089: Diff 3858.
schrockn accepted this revision.Tue, Aug 20, 8:05 PM
schrockn added a subscriber: schrockn.

per IRL discussion let's do some better names and comments because this is complicated, but looks good

This revision is now accepted and ready to land.Tue, Aug 20, 8:05 PM
alangenfeld updated this revision to Diff 3866.Tue, Aug 20, 9:00 PM

reuse solid handle to and use that to index for compute step

schrockn accepted this revision.Tue, Aug 20, 9:09 PM

ya this seems nicer

This revision was automatically updated to reflect the committed changes.