Page MenuHomePhabricator

allow optional configuration of parent pipeline solids for subsetted pipeline
ClosedPublic

Authored by alangenfeld on Oct 14 2020, 5:43 PM.

Details

Summary

This diff adds optional config schema for the solids that are not included in a given subset execution, allowing run config that would work on the original pipeline to work by ignoring the extraneous schema.

resolves #3089

Test Plan

wrote some tests

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

sandyryza edited the summary of this revision. (Show Details)

curious for @schrockn's thoughts but i think this is a reasonable amount of damage for the user upside

I think renaming optional_solids to something like ignored_solids with a comment block somewhere about why we include ignored solids would be good

more of a nit-pick but the is_required bool arg that is threaded through could maybe benefit from a more verbose name

python_modules/dagster/dagster/core/definitions/environment_configs.py
261–286

we could set the description to the top level field to something explaining whats going on "This entry is ignored, but expressed to allow easier subset execution" or whatever

python_modules/dagster/dagster/core/definitions/pipeline.py
791–792

i think this needs to be top_level_solids or something - make sure to test composites

this is good but it needs tests

python_modules/dagster/dagster/core/definitions/environment_configs.py
348–359

minor preference for building up a set of required solid names and then indexing into it within the loop, mostly because solid_is_requireds is a goofy name and the ordering constraint is easy to screw up

This revision now requires changes to proceed.Oct 19 2020, 7:20 PM
alangenfeld edited reviewers, added: sandyryza; removed: alangenfeld.
alangenfeld added a reviewer: dgibson.
alangenfeld retitled this revision from RFC: allow optional configuration of parent pipeline solids for subsetted pipeline to allow optional configuration of parent pipeline solids for subsetted pipeline.
alangenfeld edited the summary of this revision. (Show Details)

So I think we need another test case where the subset in question does *not* have a default input. That is config is required.

We should also have a test case that tests out composite solids

This revision now requires changes to proceed.Tue, Nov 10, 10:48 PM

tests added - let me know if you have others you can think of that would be good

This revision is now accepted and ready to land.Wed, Nov 11, 8:56 PM