feature: constrain value of ENVIRONMENT setting

[peterschutt]

We have a bit of logic that depends on the value of settings.AppSettings.ENVIRONMENT variable, so we probably need to make the allowed values for that more structured, at the moment it is just typed str.

I'm thinking allowed values should be "dev", "stage", "prod", "local", "test", and we can either use Literal or Enum for this.

[gazorby]

I use an Enum internally with these values as well, but can they be adapted to all projects?

[peterschutt]

To clarify, are you asking if all projects can fit into the dev/stage/prod/local/test paradigm?

What values do you use in your enum?

If a downstream app has requirements beyond those they can always create their own local settings object with a variable specific to thier needs. And we can always consider adding more environments if they are common enough.

Another way to look at it is that we are doing the wrong thing by overloading the ENVIRONMENT variable with all of this implicit functionality, and should maybe be using feature flags, or a separate MODE variable instead. But, I suspect that is over-complicating it.

[peterschutt]

I'd like to make a release soon, so LMK if you think this should be left out. I'm happy to hold off as it's not a critical issue.

[gazorby]

are you asking if all projects can fit into the dev/stage/prod/local/test paradigm?

Specifically, I wonder if one need some more values, how can a local enum be used without avoiding lib's app.ENVIRONMENT to fail because it's an unknown value from the lib's env enum?

[peterschutt]

Another approach could be:

# settings.py
class AppSettings(BaseSettings):
    ENVIRONMENT: str = "prod"
    TEST_ENVIRONMENT: str = "test"
    LOCAL_ENVIRONMENT: str = "local"

app = AppSettings()

IN_TEST_ENVIRONMENT = app.ENVIRONMENT == app.TEST_ENVIRONMENT
IN_LOCAL_ENVIRONMENT = app.ENVIRONMENT == app.LOCAL_ENVIRONMENT

something like that?

[gazorby]

I'd like to make a release soon, so LMK if you think this should be left out. I'm happy to hold off as it's not a critical issue.

Actually I think It's part of a bigger issue which is: How can we easily override any setting in downstream projects?
For instance, before #219 was merged I first tried to add this at the end of my settings.py

settings.server.RELOAD = settings.app.ENVIRONMENT == ApiEnv.LOCAL.value

But this wouldn't work as my settings.py was imported after running after the lib's run_app() was called.
So all settings cannot be overriden the same way, and if you don't how the lib use them it's more difficult to meet your own needs.

[gazorby]

But maybe It's not something we want to support as the lib is already a bit opiniated design wise, so enforcing env default values is ok.

[gazorby]

# settings.py
class AppSettings(BaseSettings):
    ENVIRONMENT: str = "prod"
    TEST_ENVIRONMENT: str = "test"
    LOCAL_ENVIRONMENT: str = "local"

app = AppSettings()

IN_TEST_ENVIRONMENT = app.ENVIRONMENT == app.TEST_ENVIRONMENT
IN_LOCAL_ENVIRONMENT = app.ENVIRONMENT == app.LOCAL_ENVIRONMENT

That looks good! I find it a bit cleaner to write if IN_TEST_ENVIRONMENT: than if settings.app.ENVIRONEMNT.value == ENV.TEST.value: and adding IN_CUSTOM_ENVIRONMENT downstream is easy too.

[peterschutt]

settings.server.RELOAD = settings.app.ENVIRONMENT == ApiEnv.LOCAL.value

isn't this just the same as:

# .env
ENVIRONMENT=local
SERVER_RELOAD=true

But maybe It's not something we want to support as the lib is already a bit opiniated design wise, so enforcing env default values is ok.

Yes, this. Configuration by environment is pretty fundamental to the pattern here.

That looks good!

OK cool, I'll back out that other commit and go with this. I think it adds enough structure:)

[gazorby]

isn't this just the same as:

# .env
ENVIRONMENT=local
SERVER_RELOAD=true

I still have to set SERVER_RELOAD=false in staging/prod . I prefer to have as much of my settings to be set programmatically when possible so I can write some tests for env behavior, and I deal with a minimal set of env vars when deploying ;)

[peterschutt]

Interesting! I've always gone the other way - in my production apps I don't have any "default" environment to force explicitly setting all values. I backed off from that stance with this library as I figured it wouldn't be a very popular design choice.

In practice, it just means that the default environment values are set in terraform config instead of the application, but that means that they can all be easily modified without having to modify the application source.

[gazorby]

Just a matter of preference I guess ;)