buckket / twtxt

Decentralised, minimalist microblogging service for hackers.
http://twtxt.readthedocs.org/en/stable/
MIT License
1.94k stars 79 forks source link

Config file sanity checking, better error reporting #73

Closed kdave closed 8 years ago

kdave commented 8 years ago

I did a stupid typo Fasle instead of False in the config. This is reported as raw python exception. It would be good to print at least the line, or eg. the expected value type. The overall config sanity could look for uknown key names.

JDurstberger commented 8 years ago

I started hacking around a bit on this issue and I am a bit confused right now, because if I get this correctly the fallback parameters should take care of this problem.

return self.cfg.getboolean("twtxt", "use_pager", fallback=False)

Link to the documentation https://docs.python.org/3/library/configparser.html#fallback-values

But still an exception gets raised. Is this a python bug?

buckket commented 8 years ago

@Altoyyr https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.getboolean The fallback is only used to fill in empty values, it does not replace an invalid value, which makes kinda sense in the "explicit is better than implicit" way of things.

timofurrer commented 8 years ago

Like @buckket already pointed out the fallback is only used if the configuration value can not be found. cpython implemented it this way: https://hg.python.org/cpython/file/3.5/Lib/configparser.py#l807

The only solution for a nicer error message would be to catch the exception in every property in the Config class which is getting a value with one of the type-specific getters.

JDurstberger commented 8 years ago

@timofurrer Yepp, got it.

This is what I am working on right now. It works pretty acutally. One of the flaws in my code is, that if an config-error is found twtxt terminates, is that ok, or do we need to use a default/fallback value of some kind?

kdave commented 8 years ago

I'd say let it terminate. If you can think of some useful hint it then print it, but otherwise I think the user would be able to resolve it manually.

JDurstberger commented 8 years ago

If PullRequest #84 gets accepted soon, it should at least be possible to modify the config before checking it, I think.

buckket commented 8 years ago

I agree with @kdave, program should terminate in case of invalid config file options.

buckket commented 8 years ago

Merged and updated.