Figure out what's wrong with config.Optional(config.DictOf)

[spladug]

DictOf does what we want without Optional but the combo confusingly always returns the default. This may just be something that we detect and say "don't do that" since it's unnecessary.

from baseplate import config

CONFIG = {
    "foo": config.Optional(
        config.DictOf(config.TupleOf(config.String)),
        default={},
    )
}

app_config = {
    "foo.bar": "a, b, c, d",
    "foo.baz": "e, f",
}

cfg = config.parse_config(app_config, CONFIG)
print(cfg.foo.bar)

prints {}.

[isugimpy]

I could be crazy here, but I'm not sure config.Optional works at all. Just for kicks, I just tried

from baseplate import config

CONFIG = {
    "foo": config.Optional(
        config.DictOf(config.TupleOf(config.String)),
        default={},
    )
}

CONFIG2 = {
    "bar": config.Optional(config.Integer, default=5)
}

app_config = {
    "foo.bar": "a, b, c, d",
    "foo.baz": "e, f",
}

app_config2 = {
    "bar": "1"
}

cfg = config.parse_config(app_config, CONFIG)
print(cfg.service_whitelist)

cfg2 = config.parse_config(app_config2, CONFIG2)
print(cfg2.service_whitelist)

and the output is

None
None

[isugimpy]

For kicks, I also tried setting the value of bar to int(1) and it raised an exception, so I think I'm doing this correctly.

[spladug]

Your tests seem to be checking service_whitelist while the config involved is bar.

Optional seems to work fine for the basic use case: https://github.com/reddit/baseplate.py/blob/master/tests/unit/config_tests.py#L233-L245

[isugimpy]

Maybe shame on me, but I was just following the example from your comment above, making the assumption that I was missing something in the backing code that auto populated service_whitelist. I also started playing with this half asleep last night, which is definitely shame on me. I'll dig in harder. Thanks for linking the tests!

[spladug]

Ah sorry, service_whitelist in the original example was me leaving bits of the original code behind.

[isugimpy]

I've pinned down the problem, just not a sane way to deal with it. Using the example provided (the one that's broken in the expected way based on the issue) and way too much tracing with pdb, I'm seeing that the problem is just that it's not drilling into the dict, because the top level key (foo) doesn't exist in the items in the actual config and SpecParser.parse() just throws the config items away because of it. As mentioned, it definitely works with straight up DictOf. The real question here is if it's worth fixing. It actually does work if you do

app_config = {
    "foo": {
        "bar": "a, b, c, d",
        "baz": "e, f"
    }
}

which is expected. It could be solved by forcing Optional to construct the dict first and validate it (which, I think, functionally would require duplicating a lot of DictOf.parse()), or by reversing SpecParser.parse() so it considers the contents of the config and tries to map them to the spec, but it's likely not worth the level of effort. Since you've already mentioned the slap on the wrist approach, would you be open to a PR to do exactly that?

[spladug]

Thanks for the writeup. Agreed that it's probably not worth making it work. A slap-on-the-wrist PR would be great.