-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a safe_get_config
#217
Comments
This could help simplify a common pattern. I'm open to it. I did look into whether there's a good existing tool or pattern for this, like xpath for XML. There is dpath, but it's a pretty heavy dependency and we're just using one possible option of it, so a new function seems fine. |
Dumping my progress so far on this: code is in https://github.com/SuffolkLITLab/docassemble-ALToolbox/tree/safe_config. I had intended for it to be used as
Internally, we can return the default whenever I also slowed down on this because we rarely have configs that are nested beyond 2 dictionaries in the config, so adding this function doesn't avoid any really gnarly code. |
We've added a lot of chained code that gets things from the config, i.e.:
This is fine, but there's an issue if people have something wrong in their config file, like this error recently in teams:
Which has to be caused by a call to
get_config("assembly line", {})
being present (so the empty dict isn't returned), but still returning null.My question is if we want a safer version, like
safe_get_config
that internally would be something like this:Making sure that we would never have a
no attribute 'get'
error in production, no matter what the config looks like. Cons are that it might be harder to determine a misconfigured server, but IMO there's enough issues with YAML and spelling and stuff that hidingNone
key-values in dictionaries shouldn't make the problem worse.The text was updated successfully, but these errors were encountered: