API configuration using pydantic-settings#4
Conversation
There was a problem hiding this comment.
Overall Assessment
Excellent refactor! Moving to pydantic-settings is a solid choice for a FastAPI project. This pattern is widely used in production and will serve you well as the app grows.
What's working well:
✅ Using SecretStr for credentials shows good security awareness
✅ The @lru_cache optimization is exactly right for FastAPI dependencies
✅ Letting pydantic handle env validation removes boilerplate and improves error messages
✅ Clean separation of concerns (config in one place, database setup in another)
Main question:
The type: ignore[call-arg] comment suggests a type checker complaint. This might be a false positive from pyright — worth investigating whether it's actually needed.
Learning notes:
This PR demonstrates several FastAPI/Python best practices that are worth explaining in interviews:
- Type-safe configuration (pydantic validates at startup, not at first use)
- Dependency caching (lru_cache prevents re-reading env vars on every request)
- Secret handling (SecretStr hides credentials in logs/errors)
The code is clean, focused, and ready to merge. Great work! 🚀
| 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( |
There was a problem hiding this comment.
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).
|
|
||
|
|
||
| @lru_cache | ||
| def get_settings() -> Settings: |
There was a problem hiding this comment.
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:
- Check if removing the comment triggers an actual error
- If pyright still complains, it might be a false positive due to how
pydantic_settingsusesmodel_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!).
| environment: str = "development" | ||
|
|
||
|
|
||
| @lru_cache |
There was a problem hiding this comment.
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.
| "DATABASE_URL environment variable is required. " | ||
| "See .env.example for setup instructions." | ||
| ) | ||
| settings = get_settings() |
There was a problem hiding this comment.
Good refactor here! Moving from manual dotenv.load_dotenv() + os.environ.get() to pydantic-settings gives you:
- Type safety — Settings fields are validated at startup
- Better error messages — If
DATABASE_URLis missing, pydantic will tell you exactly what's wrong - Testability — You can pass a custom Settings object in tests
- 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).
| 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( |
There was a problem hiding this comment.
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.
|
💰 Review cost: $0.15 |
What
Centralises app configuration using
pydantic-settings.Changes
config.pywith aSettingsclass andget_settings()cached withlru_cacheDATABASE_URLnow loaded viaSettingsSecretStrused fordatabase_urlto prevent accidental loggingload_dotenv()removed fromdatabase.py— pydantic-settings handles.envloadingechoflag in async engine now driven bysettings.environmentWhy
Single source of truth for env vars, validation at startup, and a clean foundation for future config needs.