Page MenuHomePhabricator

(new-config-parsing-7) Separate validation and mutating steps into separate passes
ClosedPublic

Authored by schrockn on Dec 15 2019, 11:43 PM.

Details

Summary

This splits out validation into two separate passes. One if the
read-only validation. The other is the "post-processing" step, that
applies defaults and also converts to python representation of enums.

This allows us to clean up the validate step even more and also a more
sensible, structured place to plug in "post-validatoion processing"
(We'll name apply_default_values to post_process_config or something
in a subsequent diff).

Users will be able to inject their own logic into the post-processing
step to do things like further validation beyond the type system (e.g.
what we currently did with custom scalars)

This also exposed a pretty problematic thing that we were actually
maintaining two code paths for managing defaults.

Depends on D1672

Test Plan

BK

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

schrockn created this revision.Dec 15 2019, 11:43 PM
schrockn updated this revision to Diff 7782.Dec 16 2019, 3:39 AM
schrockn retitled this revision from See where we are at to (new-config-parsing-7) Separate validation and mutating steps into separate passes.
schrockn edited the summary of this revision. (Show Details)
schrockn edited the test plan for this revision. (Show Details)
schrockn added reviewers: alangenfeld, nate.

upmessage

alangenfeld accepted this revision.Dec 16 2019, 5:35 PM
alangenfeld added inline comments.
python_modules/dagster/dagster/core/execution/results.py
70–71

nit: solid_name -> handle? I think this gets slurped up in the API docs so worth being precise i think

python_modules/dagster/dagster/core/types/config/evaluator/validate.py
292

nit: meh on the name process but don't really have a suggestion thats substantially better

This revision is now accepted and ready to land.Dec 16 2019, 5:35 PM
schrockn updated this revision to Diff 7847.Dec 16 2019, 11:47 PM

better arg name and docs