Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 22 additions & 0 deletions backend/app/config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from functools import lru_cache

from pydantic import Field, SecretStr
from pydantic_settings import BaseSettings, SettingsConfigDict


class Settings(BaseSettings):
model_config = SettingsConfigDict(env_file=".env")
# no default value for database_url, it must be provided via environment variable or .env file
database_url: SecretStr = Field(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great use of SecretStr! This prevents the database URL (which contains credentials) from being accidentally logged or printed. When you call .get_secret_value() in database.py, you're explicitly acknowledging you need the raw value.

One thing to know: SecretStr doesn't encrypt anything — it just hides the value in logs and repr() output. The actual security comes from keeping the .env file out of version control (already done via .gitignore).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Future enhancement idea (not blocking): If you want to validate that database_url actually looks like a PostgreSQL URL, you could use pydantic's PostgresDsn type:

from pydantic import PostgresDsn

class Settings(BaseSettings):
    database_url: PostgresDsn = Field(...)
    # ...

This would catch typos like postgres:// vs postgresql:// at startup. But SecretStr works fine too — just a tradeoff between stricter validation vs. keeping credentials hidden in error messages.

description="Database URL in the format: postgresql://user:password@host:port/database",
)
environment: str = "development"


@lru_cache

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice use of @lru_cache! This ensures Settings() is only instantiated once, even if get_settings() is called many times.

For context: FastAPI dependencies like this are called on every request, so without caching you'd be re-reading env vars constantly. The cache makes this essentially free after the first call.

One small note: in Python 3.9+, you can use @lru_cache without parentheses (it's equivalent to @lru_cache(maxsize=128)), but @lru_cache() is totally fine and arguably clearer about intent.

def get_settings() -> Settings:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Question about the type: ignore: This suggests pyright is complaining about calling Settings() with no arguments.

In my experience, this shouldn't be necessary — BaseSettings is designed to work without arguments (it loads from env vars automatically). A few things to try:

  1. Check if removing the comment triggers an actual error
  2. If pyright still complains, it might be a false positive due to how pydantic_settings uses model_config

If you do need the ignore, consider making it more specific: type: ignore[call-arg] is fine, but a brief inline comment explaining why would help future maintainers (including you in 6 months!).

# database_url does not have a default value, so if it's missing from the environment variables or .env file,
# Pydantic will raise a validation error here, which is desirable to catch configuration issues
# The type ignore below is for static type checkers (e.g., mypy) because 'database_url' has no default value,
# but this is safe since Pydantic will enforce its presence at runtime.
return Settings() # type: ignore[call-arg]
17 changes: 5 additions & 12 deletions backend/app/database.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,15 @@
import os
from collections.abc import AsyncGenerator

from dotenv import load_dotenv
from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine
from sqlmodel import SQLModel

load_dotenv()
from .config import get_settings

DATABASE_URL = os.environ.get("DATABASE_URL")
if not DATABASE_URL:
raise ValueError(
"DATABASE_URL environment variable is required. "
"See .env.example for setup instructions."
)
settings = get_settings()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good refactor here! Moving from manual dotenv.load_dotenv() + os.environ.get() to pydantic-settings gives you:

  1. Type safety — Settings fields are validated at startup
  2. Better error messages — If DATABASE_URL is missing, pydantic will tell you exactly what's wrong
  3. Testability — You can pass a custom Settings object in tests
  4. Less boilerplate — No need to check if not DATABASE_URL

The old explicit check was good defensive coding, but pydantic's validation is more comprehensive (it also checks types, not just presence).


engine = create_async_engine(
DATABASE_URL, echo=os.environ.get("ENVIRONMENT") == "development"
)
DATABASE_URL = settings.database_url.get_secret_value()

engine = create_async_engine(DATABASE_URL, echo=settings.environment == "development")

# keeps objects usable after commit (relevant for async sessions)
_session_factory = async_sessionmaker(engine, expire_on_commit=False)
Expand Down
4 changes: 2 additions & 2 deletions backend/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ dependencies = [
"fastapi[standard]>=0.115",
"sqlmodel>=0.0.22",
"asyncpg>=0.30",
"python-dotenv>=1.0",
"pydantic-settings>=2.14.1",
]
[tool.ruff]
target-version = "py313"
Expand Down Expand Up @@ -71,4 +71,4 @@ venv = ".venv"

[tool.pytest.ini_options]
asyncio_mode = "auto"
testpaths = ["tests"]
testpaths = ["tests"]