Page MenuHomeElementl

fix optional solid config for input config
ClosedPublic

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

Details

Summary

Ran in to this dogfooding,

Test Plan

added a test

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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

python_modules/dagster/dagster/core/definitions/environment_configs.py
57–63

[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

159

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.Thu, Apr 8, 11:19 PM