Page MenuHomeElementl

fix optional solid config for input config

Authored by alangenfeld on Apr 7 2021, 10:04 PM.



Ran in to this dogfooding,

Test Plan

added a test

Diff Detail

R1 dagster
opt-bug (branched from master)
Lint Passed
No Test Coverage

Event Timeline

I'm finding this diff a little difficult to parse - the test name refers to unsatisfied inputs, and the changes are to get_inputs_field, but the test body uses regular solid config, not solid input config?

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

ya let me take another pass at this

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

more tests and comments


[1] would we have to like build the whole config for the parent pipeline and then build the ignored section by grabbing the entries out of the parent config


This is the root of the problem - the dep structure just doesn't have anything for the ignored solids so we always think they are unsatisfied inputs which will trigger config to be added.

Unfortunately - just dropping this section of config if the solid is ignored doesn't work for the case of making sure that the config for a solid that *does* use the input loading stuff is properly ignored. I added test cases for this below.

Allowing the extra config to get generated but just marking it as ignored seems to work for the most cases though its a bit strange. It's not clear to me how much surgery would need to happen to have all the context to this "correctly" [1]

Ok I think I now understand 85% of what's going on here. Going to settle with that and trust you on the last 15%.

This revision is now accepted and ready to land.Apr 8 2021, 11:19 PM