Idea looks good, implementation looks good, several small issues in terms of polishing the branch for merging. [11] +description and an optional default value. Additionally type may be s/type/'type'/ [12] + config = ConfigOptions() + content = None The second line may be dropped. [13] + "options": Dict(String(), + OneOf( + # regex type requires validator + KeyDict({"type": Constant("regex"), Indentation should be revised here. Feel free to define an auxiliary variable externally. [14] + "validator": String(), We need a new RegEx type in the schema to validate the regular expression. [15] + optional=["default", "description",]), s/",]/"]/ Same on the bottom one. [16] + @classmethod + def load_file(cls, pathname, optional=False): Drop the @classmethod here. load() doesn't have it, and MetaData doesn't use it either, so let's make the interface even. Also, please rename the functions to conform to the same conventions established by MetaData: parse() and load(), the former with a data _string_, the latter with a filename. Add parse_serialization_data to handle the dict case. The behavior of these functions should also mimic MetaData: load() raises FileNotFound, and so on. These two classes are exactly alike. There's no reason for us to diverge and then having to remind of which interface does what, which load accepts a dict, etc. [17] + def validate(self, options): + """Validate options using the file at `path`. Which path? [18] + This method will return a dict with options that have passed + validation. This means options not declared in the formula + metadata will trigger a ServiceConfigError exception. + + This method includes defaults for options which have no values + supplied if such defaults are defined in the metadata. Please reword: This method validates all the provided options, and returns a new dictionary with values properly typed and default values set for missing keys. If a provided option is unknown or its value fails validation, ServiceConfigError is raised. [19] + "%s is not a valid configuration option." % (option)) (...) + "a normal Python regex pattern for %s" % ( + option)) (...) + "See http://docs.python.org/library/re.html" % ( + option)) (...) + raise ServiceConfigError("Invalid options specification: %s" % ( + error)) This is a tuple: (option,) This is an expression with redundant parenthesis: (option) Drop the parenthesis in all of those cases. [20] + validator = validation_kinds[kind] + if kind == "regex": + pattern = self._data[option].get("validator") The interface for validators can deal with such distinctions so that we don't have to special case them in the generic function. E.g.: validator = validation_kinds[kind] value, valid = validator(value, self._data[option]) [21] +class ServiceConfigError(Exception): + """Indicates an issue related to service options.""" + pass Drop pass. [22] +def regex_validator(str, pat): + return re.match(pat, str) is not None Don't your eyes bleed and pop with the obvious inversion of parameters? I can't emphasise enough how important consistency is for the long term health of a software project. It's _key_ to pay attention to details such as parameter consistency, interface consistency across similar classes, and so on. Those are the details that will make our lives (or someone else's) less painful when giving maintenance in years to come. [23] + "float": float, + "regex": regex_validator Let's have proper validators, with an even interface on all of them, as per [20]. Alright, I think with that we can merge it.