Revisit config validation

Description

The only type of validation at this point is DriverOption.required(), which is enforced when building the DriverConfig.

But it is a bit clunky: a lot of the things that are marked required are not really, if you override the corresponding component manually by subclassing the DriverContext.

On the other hand, there are other types of validations that would be nice to have: protecting against silly values (e.g. reconnection interval of 10ms), rules that involve multiple options, warn for options that are never read, etc.

The simplest approach is to validate each value when we extract it, but that can happen late in the initialization process.

Environment

None

Pull Requests

None

Activity

Show:

Olivier Michallat 
April 17, 2018 at 11:26 PM
(edited)

I believe it is possible to consider all options as required

This wouldn't play well with the high degree of customization allowed by the driver.

For example, different implementations of the same component use different options, see request.throttler for example. Forcing all possible options of all possible implementations to be defined is not a great user experience; for example, someone who logs their configuration at startup with will see a bunch of stuff they don't care about. Also, this could create backward compatibility issues if we add new options in the future, and someone does not inherit from the bundled reference.conf (not that unlikely if they bypass Typesafe config altogether).

Very few options are really mandatory, depending on how far you go. For example if I extend DefaultDriverContext to inject a request throttler implementation programmatically, request.throttler.class itself becomes optional. That's also what makes validation complicated. At this point, I don't see a better approach than validating from each component's constructor, and making sure exceptions are propagated cleanly.

Alex Dutra 
March 1, 2018 at 2:53 PM

I would also like to suggest a different approach to required/optional.

TypeSafe config actually is designed around null-safety and doesn't really like when a configuration key is not present, or is null.

Instead of checking if some option is present or not, I believe it is possible to consider all options as required. If some options do not have a sensible default, it is always possible to express that "non-value" differently, e.g. by introducing a special value (-1 if the value is expected to be a positive integer, or an enum constant like UNDEFINED, etc.) and/or by splitting the option into two parts: a boolean key that determines if the option is enabled or not, then a separate key that carries the option value when enabled.

One of the advantages of going down this path is that it makes introspection possible. TypeSafe Config has all the tooling required to introspect options and gather information about them: their types, the description associated with them, and their origin (the exact file where the option was located). Leveraging these tools would allow us to generate documentation automatically from the reference.conf files themselves, which imo would be a great improvement on the documentation front. But this, of course, would only be possible if all options are present.

Fixed

Details

Assignee

Reporter

Fix versions

Priority

Created July 19, 2017 at 12:49 AM
Updated August 22, 2018 at 2:30 PM
Resolved July 16, 2018 at 5:13 PM