sts-sponsors team mailing list archive

[Thorsten Merten <mp+441473@xxxxxxxxxxxxxxxxxx>]

Review: Approve

TIL - BaseSettings. Nice, also nice that you have Fields in here. Thought for the future inline.

Diff comments:

> diff --git a/backend/msm/user_api/_setup.py b/backend/msm/user_api/_setup.py
> index 03b9d18..29d8437 100644
> --- a/backend/msm/user_api/_setup.py
> +++ b/backend/msm/user_api/_setup.py
> @@ -8,25 +7,14 @@ from fastapi.middleware.cors import CORSMiddleware
>  from . import _base
>  from .. import PACKAGE
>  from ..db import Database
> +from ..settings import SETTINGS
>  
> -POSTGRES_HOST = environ.get("POSTGRES_HOST")
> -POSTGRES_PORT = environ.get("POSTGRES_PORT")
> -POSTGRES_DB = environ.get("POSTGRES_DB")
> -POSTGRES_USER = environ.get("POSTGRES_USER")
> -POSTGRES_PASSWORD = environ.get("POSTGRES_PASSWORD")
> -DEFAULT_DB_DSN = (
> -    "postgresql+asyncpg://"
> -    + f"{POSTGRES_USER}:{POSTGRES_PASSWORD}@{POSTGRES_HOST}/{POSTGRES_DB}"
> -)
>  
> -# TODO: make config dynamic and allow env vars
> -origins = [
> -    "http://localhost:8405";,
> -    "http://127.0.0.1:8405";,
> -]
> +def create_app(db_dsn: str | None = None) -> FastAPI:

Passing the string in here is needed for the tests, right? Otherwise it would be nice to get rid of that, too.

> +    """Create the FastAPI WSGI application."""
> +    if db_dsn is None:
> +        db_dsn = str(SETTINGS.db_dsn)
>  
> -
> -def create_app(db_dsn: str = DEFAULT_DB_DSN) -> FastAPI:
>      @asynccontextmanager
>      async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
>          await db.connect()


-- 
https://code.launchpad.net/~ack/maas-site-manager/+git/site-manager/+merge/441473
Your team MAAS Committers is subscribed to branch ~ack/maas-site-manager:settings-module.