Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment: multi-environment configuration in Python #161

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions app/src/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
# https://docs.python.org/3/library/__main__.html

import logging
import os

import src.app
import src.config
import src.logging
from src.app_config import AppConfig
from src.util.local import load_local_env_vars
Expand All @@ -16,23 +18,26 @@


def main() -> None:
load_local_env_vars()
app_config = AppConfig()
config = src.config.load(environment_name=os.getenv("ENVIRONMENT", "local"), environ=os.environ)

app = src.app.create_app()
app = src.app.create_app(config)
logger.info("loaded configuration", extra={"config": config})

environment = app_config.environment
all_configs = src.config.load_all()
logger.info("loaded all", extra={"all_configs": all_configs})
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code should move to a unit test, where it checks that each one is an instance of RootConfig.


environment = config.app.environment

# When running in a container, the host needs to be set to 0.0.0.0 so that the app can be
# accessed from outside the container. See Dockerfile
host = app_config.host
port = app_config.port
host = config.app.host
port = config.app.port

logger.info(
"Running API Application", extra={"environment": environment, "host": host, "port": port}
)

if app_config.environment == "local":
if config.app.environment == "local":
# If python files are changed, the app will auto-reload
# Note this doesn't have the OpenAPI yaml file configured at the moment
app.run(host=host, port=port, use_reloader=True, reloader_type="stat")
Expand Down
18 changes: 8 additions & 10 deletions app/src/adapters/db/clients/postgres_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import sqlalchemy.pool as pool

from src.adapters.db.client import DBClient
from src.adapters.db.clients.postgres_config import PostgresDBConfig, get_db_config
from src.adapters.db.clients.postgres_config import PostgresDBConfig

logger = logging.getLogger(__name__)

Expand All @@ -19,9 +19,7 @@ class PostgresDBClient(DBClient):
as configured by parameters passed in from the db_config
"""

def __init__(self, db_config: PostgresDBConfig | None = None) -> None:
if not db_config:
db_config = get_db_config()
def __init__(self, db_config: PostgresDBConfig) -> None:
self._engine = self._configure_engine(db_config)

if db_config.check_connection_on_init:
Expand Down Expand Up @@ -82,17 +80,17 @@ def check_db_connection(self) -> None:
def get_connection_parameters(db_config: PostgresDBConfig) -> dict[str, Any]:
connect_args = {}
environment = os.getenv("ENVIRONMENT")
if not environment:
raise Exception("ENVIRONMENT is not set")

if environment != "local":
connect_args["sslmode"] = "require"
# if not environment:
# raise Exception("ENVIRONMENT is not set")
#
# if environment != "local":
# connect_args["sslmode"] = "require"

return dict(
host=db_config.host,
dbname=db_config.name,
user=db_config.username,
password=db_config.password,
password=db_config.password.get_secret_value(),
port=db_config.port,
options=f"-c search_path={db_config.db_schema}",
connect_timeout=3,
Expand Down
24 changes: 3 additions & 21 deletions app/src/adapters/db/clients/postgres_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import logging
from typing import Optional

import pydantic
from pydantic import Field

from src.util.env_config import PydanticBaseEnvConfig
Expand All @@ -13,26 +14,7 @@ class PostgresDBConfig(PydanticBaseEnvConfig):
host: str = Field("localhost", env="DB_HOST")
name: str = Field("main-db", env="POSTGRES_DB")
username: str = Field("local_db_user", env="POSTGRES_USER")
password: Optional[str] = Field(..., env="POSTGRES_PASSWORD")
password: Optional[pydantic.types.SecretStr] = Field(None, env="POSTGRES_PASSWORD")
db_schema: str = Field("public", env="DB_SCHEMA")
port: str = Field("5432", env="DB_PORT")
port: int = Field(5432, env="DB_PORT")
hide_sql_parameter_logs: bool = Field(True, env="HIDE_SQL_PARAMETER_LOGS")


def get_db_config() -> PostgresDBConfig:
db_config = PostgresDBConfig()

logger.info(
"Constructed database configuration",
extra={
"host": db_config.host,
"dbname": db_config.name,
"username": db_config.username,
"password": "***" if db_config.password is not None else None,
"db_schema": db_config.db_schema,
"port": db_config.port,
"hide_sql_parameter_logs": db_config.hide_sql_parameter_logs,
},
)

return db_config
6 changes: 3 additions & 3 deletions app/src/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@
logger = logging.getLogger(__name__)


def create_app() -> APIFlask:
def create_app(config) -> APIFlask:
app = APIFlask(__name__)

src.logging.init(__package__)
src.logging.init(__package__, config.logging)
flask_logger.init_app(logging.root, app)

db_client = db.PostgresDBClient()
db_client = db.PostgresDBClient(config.database)
flask_db.register_db_client(db_client, app)

configure_app(app)
Expand Down
2 changes: 1 addition & 1 deletion app/src/app_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@


class AppConfig(PydanticBaseEnvConfig):
environment: str
environment: str = "unknown"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe environment: Optional[str] = None? That might be annoying with the linter though


# Set HOST to 127.0.0.1 by default to avoid other machines on the network
# from accessing the application. This is especially important if you are
Expand Down
39 changes: 39 additions & 0 deletions app/src/config/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
#
# Multi-environment configuration expressed in Python.
#

import importlib
import logging
import pathlib

from src.adapters.db.clients.postgres_config import PostgresDBConfig
from src.app_config import AppConfig
from src.logging.config import LoggingConfig
from src.util.env_config import PydanticBaseEnvConfig

logger = logging.getLogger(__name__)


class RootConfig(PydanticBaseEnvConfig):
app: AppConfig
database: PostgresDBConfig
logging: LoggingConfig


def load(environment_name, environ=None) -> RootConfig:
"""Load the configuration for the given environment name."""
logger.info("loading configuration", extra={"environment": environment_name})
module = importlib.import_module(name=".env." + environment_name, package=__package__)
config = module.config

if environ:
config.override_from_environment(environ)
config.app.environment = environment_name

return config


def load_all() -> dict[str, RootConfig]:
"""Load all environment configurations, to ensure they are valid. Used in tests."""
directory = pathlib.Path(__file__).parent / "env"
return {item.stem: load(str(item.stem)) for item in directory.glob("*.py")}
26 changes: 26 additions & 0 deletions app/src/config/default.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#
# Default configuration.
#
# This is the base layer of configuration. It is used if an environment does not override a value.
# Each environment may override individual values (see local.py, dev.py, prod.py, etc.).
#
# This configuration is also used when running tests (individual test cases may have code to use
# different configuration).
#

from src.adapters.db.clients.postgres_config import PostgresDBConfig
from src.app_config import AppConfig
from src.config import RootConfig
from src.logging.config import LoggingConfig


def default_config():
return RootConfig(
app=AppConfig(),
database=PostgresDBConfig(),
logging=LoggingConfig(
format="json",
level="INFO",
enable_audit=True,
),
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this require any process that runs to be able to load all environment variables? For example, if I have a config for connecting to a separate service, and it's only used in a few select processes, would I put that here in the default config? Let's assume that I don't want to set defaults in the config and want it to error if any fields aren't set.

9 changes: 9 additions & 0 deletions app/src/config/env/dev.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#
# Configuration for dev environments.
#
# This file only contains overrides (differences) from the defaults in default.py.
#

from .. import default

config = default.default_config()
16 changes: 16 additions & 0 deletions app/src/config/env/local.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#
# Configuration for local development environments.
#
# This file only contains overrides (differences) from the defaults in default.py.
#

import pydantic.types

from .. import default

config = default.default_config()

config.database.password = pydantic.types.SecretStr("secret123")
config.database.hide_sql_parameter_logs = False
config.logging.format = "human_readable"
config.logging.enable_audit = False
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would there be a way to have this check for a file named local_overrides.py and use that? I'm thinking about local testing where I want to tinker with environment variables, but not check them in - especially when working with passwords or keys to access services. We could add this local_overrides.py file to the .gitignore.

Adding something like this to the end of this file:

if os.path.exists("local_overrides.py"):
     importlib.import_module("local_overrides") # whatever the right path/params are

9 changes: 9 additions & 0 deletions app/src/config/env/prod.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#
# Configuration for prod environment.
#
# This file only contains overrides (differences) from the defaults in default.py.
#

from .. import default

config = default.default_config()
9 changes: 9 additions & 0 deletions app/src/config/env/staging.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#
# Configuration for staging environment.
#
# This file only contains overrides (differences) from the defaults in default.py.
#

from .. import default

config = default.default_config()
4 changes: 2 additions & 2 deletions app/src/logging/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,5 @@
import src.logging.config as config


def init(program_name: str) -> config.LoggingContext:
return config.LoggingContext(program_name)
def init(program_name: str, logging_config: config.LoggingConfig) -> config.LoggingContext:
return config.LoggingContext(program_name, logging_config)
33 changes: 24 additions & 9 deletions app/src/logging/config.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import enum
import logging
import os
import platform
import pwd
import sys
from typing import Any, ContextManager, cast

import pydantic

import src.logging.audit
import src.logging.formatters as formatters
import src.logging.pii as pii
Expand All @@ -15,15 +18,27 @@
_original_argv = tuple(sys.argv)


class LoggingFormat(str, enum.Enum):
json = "json"
human_readable = "human_readable"


class HumanReadableFormatterConfig(PydanticBaseEnvConfig):
message_width: int = formatters.HUMAN_READABLE_FORMATTER_DEFAULT_MESSAGE_WIDTH


class LoggingConfig(PydanticBaseEnvConfig):
format = "json"
level = "INFO"
enable_audit = True
human_readable_formatter = HumanReadableFormatterConfig()
format: LoggingFormat = LoggingFormat.json
level: str = "INFO"
enable_audit: bool = True
human_readable_formatter: HumanReadableFormatterConfig = HumanReadableFormatterConfig()

@pydantic.validator("level")
def valid_level(cls, v):
value = logging.getLevelName(v)
if not isinstance(value, int):
raise ValueError("invalid logging level %s" % v)
return v

class Config:
env_prefix = "log_"
Expand Down Expand Up @@ -58,8 +73,8 @@ class LoggingContext(ContextManager[None]):
and calling this multiple times before exit would result in duplicate logs.
"""

def __init__(self, program_name: str) -> None:
self._configure_logging()
def __init__(self, program_name: str, config: LoggingConfig) -> None:
self._configure_logging(config)
log_program_info(program_name)

def __enter__(self) -> None:
Expand All @@ -72,14 +87,14 @@ def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None:
# of those tests.
logging.root.removeHandler(self.console_handler)

def _configure_logging(self) -> None:
def _configure_logging(self, config: LoggingConfig) -> None:
"""Configure logging for the application.

Configures the root module logger to log to stdout.
Adds a PII mask filter to the root logger.
Also configures log levels third party packages.
"""
config = LoggingConfig()
# config = LoggingConfig() # TODO inject

# Loggers can be configured using config functions defined
# in logging.config or by directly making calls to the main API
Expand Down Expand Up @@ -112,7 +127,7 @@ def get_formatter(config: LoggingConfig) -> logging.Formatter:
The formatter is determined by the environment variable LOG_FORMAT. If the
environment variable is not set, the JSON formatter is used by default.
"""
if config.format == "human-readable":
if config.format == LoggingFormat.human_readable:
return get_human_readable_formatter(config.human_readable_formatter)
return formatters.JsonFormatter()

Expand Down
34 changes: 24 additions & 10 deletions app/src/util/env_config.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
import os
import logging

from pydantic import BaseSettings
from pydantic import BaseModel

import src
logger = logging.getLogger(__name__)

env_file = os.path.join(
os.path.dirname(os.path.dirname(src.__file__)),
"config",
"%s.env" % os.getenv("ENVIRONMENT", "local"),
)

class PydanticBaseEnvConfig(BaseModel):
"""Base class for application configuration.

Similar to Pydantic's BaseSettings class, but we implement our own method to override from the
environment so that it can be run later, after an instance was constructed."""

class PydanticBaseEnvConfig(BaseSettings):
class Config:
env_file = env_file
validate_assignment = True
Comment on lines -14 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could require a prefix option here too to provide some namespacing for the various configs, e.g., have the PostgresConfig environment variables all start with a PG_DB_ prefix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The implementation on PFML does this)


def override_from_environment(self, environ, prefix=""):
"""Recursively override field values from the given environment variable mapping."""
for name, field in self.__fields__.items():
if field.is_complex():
# Nested models must be instances of this class too.
getattr(self, name).override_from_environment(environ, prefix=name + "_")
continue

env_var_name = field.field_info.extra.get("env", prefix + name)
for key in (env_var_name, env_var_name.lower(), env_var_name.upper()):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to be stricter here and require a certain casing, avoid letting someone accidentally set the environment variable in lowercase and then not understand why it's not being overridden if it's also set in uppercase elsewhere.

if key in environ:
logging.info("override from environment", extra={"key": key})
setattr(self, field.name, environ[key])
break
Loading