Page MenuHomePhabricator

(dict-where-type-is-used-1) Allow usage of raw dict where config_type is expected; improve and simplify error handling

Authored by schrockn on Jan 8 2020, 6:44 PM.



This diff does two main things.

  1. Allows the more condense config type definition syntax anywhere where

resolve_to_config_type is called. This includes the arguments to
Field(), where previously one had to use the proper Dict type. The goal
is here is so that end users really never have to use Dict and we can
rename that to Shape and divorce ourselves from the runtime names

  1. Simplify error handling there. This is both an improvement and a

regression. This gets rid of the some of the context-specific error messages.
There is instead a generic error path for all config type conversions.
This now includes a stack of where you are in the config spec.

Removing the context-specific error messaging simplifies the code quite
a bit. I think that error message in combination with the stack trace
is sufficient to debug cases, but I could be persuaded that is wrong.

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 edited the summary of this revision. (Show Details)
schrockn added reviewers: alangenfeld, max.


for the error messages I agree that we can tolerate losing the bit of context since the stack trace should point you in the right direction - but I think we can do better than whats in the diff as it stands


this should probably just go in the error message - will make learning much better for new users


cannot be converted to type

ya reads a little weird to me - I think it moving the rules in to the error message and tweaking this to "cannot be resolved, the value can be a:" (followed by rules) or something would be better


seeing this reenforces above comments - I think I would be puzzled for a bit if i hit this for the first time

This revision now requires changes to proceed.Jan 9 2020, 10:21 PM

alex feedback on error messages

alangenfeld added inline comments.

just put inline if its not re-used?


remove the rst markup for the text in the error message not the doc block

This revision is now accepted and ready to land.Jan 9 2020, 10:59 PM

meh don't want to deal with indenting




will address in next PR