Page MenuHomePhabricator

Eliminate selector argument from PipelineDefinition

Authored by schrockn on Nov 27 2019, 11:09 PM.



This argument is actually unnecessary because the selector property
can be derived from the solid definitions and the dependency graph. The
only code path that populates this is through build_sub_pipeline and that
codepath subsets the dependencies and the solid definitions. From this
information you can construct the set of solid instances that will execute

The current PipelineDefinition allowed for the pipeline to be constructed
in an invalid state, where the ExecutionSelection can potentially "lie" about
what will be executed, or vice versa.

This state of the world might be a bit awkward. (The constructing
a new PipelineDefinition that is a subset has always been a bit strange)

However post this diff the world is more consistent and the API
less error-prone

Test Plan


Diff Detail

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

Event Timeline

schrockn created this revision.Nov 27 2019, 11:09 PM
schrockn updated this revision to Diff 6971.Nov 27 2019, 11:35 PM
schrockn edited the summary of this revision. (Show Details)


schrockn updated this revision to Diff 6987.Nov 28 2019, 12:54 AM


schrockn updated this revision to Diff 6988.Nov 28 2019, 12:56 AM


prha accepted this revision.Dec 2 2019, 5:13 PM

This looks good to me...

Also has the added benefit of making the signature match the described arguments...


remove comment

This revision is now accepted and ready to land.Dec 2 2019, 5:13 PM
This revision was automatically updated to reflect the committed changes.