Env custom type casting

[ahopkins]

Inspired by the pattern from @Varriount in #2321 and app.router.register_pattern, this PR adds the ability to register custom converters to cast values loaded from environment variables into a custom type.

Example:

class Foo:
    def __init__(self, name) -> None:
        self.name = name


config = Config(converters=[Foo])
app = Sanic(__name__, config=config)

@app.before_server_start
async def display(app, _):
    assert isinstance(app.config.FOO, Foo)

[realDragonium]

looking good 👍

[adarsharegmi]

better

[prryplatypus]

Was just thinking about this... I think it would be a good idea to pass a "key" parameter to the converter, to make it possible to convert only a specific environment variable without converting others that would also be possible to convert.

[ahopkins]

Was just thinking about this... I think it would be a good idea to pass a "key" parameter to the converter, to make it possible to convert only a specific environment variable without converting others that would also be possible to convert.

Can you provide a snippet of what that would look like as a dev? At that point, why not just convert yourself?

[prryplatypus]

Sure! That's what I thought this PR was meant to avoid, so this came to mind.

In one of my codebases I currently have the following method, which is called every time the application is started. It's only use is to coerce values.

## config.py
from base64 import b64decode
from sanic import Sanic

def init(app: Sanic) -> None:
    """This is simply used to coerce the config values
    into the correct type where necessary. Use environment
    variables to set config values.
    """
    config = app.config
    if isinstance(config.JWT_SECRET, str):
        config.JWT_SECRET = b64decode(config.JWT_SECRET)

After thinking about it, I realised that passing the key may not be possible as there'd be no way to indicate "no, I don't want to coerce this", but the following implementation came to mind to replace the example above, where the value can be any callable:

config = Config(coercers={"JWT_SECRET": b64decode})
app = Sanic(__name__, config=config)

@app.before_server_start
async def display(app, _):
    assert isinstance(app.config.JWT_SECRET, bytes)

[ahopkins]

Hmm.... 🤔 two thoughts:

1. I would think another name other than coercer. It might be confusing to have converter & coercer. Will have to think on that.
2. It probably should be reversed, with the type being the key so it could be used on multiple:

Config(something={b64decode: "JWT_SECRET", foobar: ["FOO", "BAR"]})

Thinking more, maybe the answer is to just combine both concepts.

class Foo:
    def __init__(self, name) -> None:
        self.name = name


config = Config(converters=[
    Foo,
    (b64decode, "JWT_SECRET"),
    (foobar, "FOO", "BAR"),
])

# OR

config = Config(converters=[
    Foo,
    (b64decode, "JWT_SECRET"),
    (foobar, ("FOO", "BAR")),
])

# OR

config = Config(converters={
    Foo: None,
    b64decode: "JWT_SECRET",
    foobar: ("FOO", "BAR"),
})

I think the third option may be the winner. Pass a dict (maybe we also allow a simple list like in the original if NONE of them have key constraints) where:

• if value is None, then apply to all keys and see what matches (current implementation)
• if value is string, then apply to single key
• if value is list, then apply to those keys only

[prryplatypus]

I like it, either option works for me! What about if a key appears in more than one place though? That's why I suggested using they keys as key (maybe we can make it such that a None key acts as a "match all"?)

[ahopkins]

What about if a key appears in more than one place though?

I think it executes in the order they are listed. If there are multiple things that run on it, they would each run in turn.

[Varriount]

Looking at the code, it appears that converters are (more or less) global - Config.register_type, despite being an instance method, modifies an instance of CastRegistry that is bound to a class attribute.

I feel that this is very misleading, as instance methods do not normally modify class-level or global data. Rather than have a shared registry, it would probably be better to just have a registered_converters instance attribute that can be modified to manipulate registered converters.

[ahopkins]

it would probably be better to just have a registered_converters instance attribute

That makes total sense. Nice call out.

[Varriount]

(thinking on it, the attribute would probably be better named as just converters or some such)

Some other thoughts:

• If the logic is changed to use an instance attribute, a decision needs to be made on how users are going to manipulate that attribute. It's tempting to just expose it directly, since this would give users the most control (for example, by inserting a converter at a specific point), however it may pose a problem for any future changes.
• I think a decision may eventually need to be made on how "complex" the built-in configuration logic is going to be. To be clear, I do think that this PR is a good addition, as it lets users load types like dates, json, etc. from the environment. Configuration loading, however, is one of those mechanisms that can become quite extensive, and as such it's important to make good architecture decisions early on.
• One of the most flexible configuration systems I've ever used was the one supported by Spring Boot, though it was also frustrating too - primarily because it was difficult to tell where it was looking for data, and where certain configuration values had originated.

[ahopkins]

• however it may pose a problem for any future changes.

It really needs to be done upfront since environment variables are loaded when the application instance is instantiated. After that, there really is not so much use to them.

This could be a possibility, but it is verbose.

config = Config()
config.converters = [...]

• it lets users load types like dates, json, etc

datetime and UUID are the two I have in mind most. Originally I was thinking these should just be added, but that would be a breaking change for sure. For someone that wants this, it seems easy enough to do this:

app = Sanic(..., config=Config(converters=[datetime, UUID]))

That was what I figured the most common use case would be.

• Configuration loading, however, is one of those mechanisms that can become quite extensive, and as such it's important to make good architecture decisions early on.

💯

• was the one supported by Spring Boot

Now I need to go dust off my skills and clear the cobwebs from my brain. Its been a while. Will need to go see how they do it. Something in particular that sticks out for you?

---

Another option is that we keep this PR much more limited akin to what it is doing now. Then move any further sophistication over to Sanic Extensions where I think we have more liberty to be cute with things.

from sanic_ext import ConfigPlus

config = ConfigPlus()
# do whatever needs to be done
app = Sanic(..., config=config)