Skip to content

feat: add structured settings management (SBP-531) - #132

Open
marius-mather wants to merge 32 commits into
devfrom
feat/settings
Open

feat: add structured settings management (SBP-531)#132
marius-mather wants to merge 32 commits into
devfrom
feat/settings

Conversation

@marius-mather

@marius-mather marius-mather commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Summary

SBP-531: we have multiple environment variables scattered throughout the code, use pydantic-settings to bring them together in a structured way. Benefits:

  • Better visibility of all the settings/variables we use
  • Better parsing/validation, validation only has to be done once rather than repeatedly
  • Easier testing through overriding the settings objects/FastAPI dependencies

Changes

  • Implement settings objects with pydantic-settings: reads from either .env file or environment variables, does parsing and validation
  • Replace any environment variable access with settings objects
  • Use a get_settings dependency to ensure FastAPI fetches and caches the settings
  • Rework tests to use mocked/overridden settings - reduces test setup

How to Test

Run uv run pytest

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added or updated documentation where necessary
  • I have run linting and unit tests locally
  • The code follows the project's style guidelines

@vtnphan vtnphan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's quite big to be review in details, just got one small comment in prerun env as we're gonna remove it soon. The others look good to me. Thank you

Comment on lines +35 to +42
def _aws_prerun_env(settings: Settings) -> dict[str, str]:
return {
"AWS_ACCESS_KEY_ID": settings.aws.access_key_id,
"AWS_SECRET_ACCESS_KEY": settings.aws.secret_access_key,
"AWS_REGION": settings.aws.region,
}


Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we can skip this as we'll remove the AWS keys in prerun_script in the next PR soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants