feat: Implement async PostgreSQL database connection #65
Conversation
WalkthroughAdds PostgreSQL environment keys, a new async PostgreSQL connection pool manager (asyncpg) with lifecycle helpers, unit tests for the connection API, pytest configuration for async tests, and two new dependencies in requirements. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant DC as DatabaseConnection
participant Pool as asyncpg.Pool
participant DB as PostgreSQL
Note over App,DC: Startup / test setup
App->>DC: initialize_db_connection()
activate DC
DC->>DC: connect() [classmethod]
DC->>Pool: asyncpg.create_pool(...)
Pool->>DB: establish connections
DC->>DC: store pool reference
deactivate DC
Note over App,Pool: Runtime usage
App->>DC: get_connection()
activate DC
DC->>Pool: acquire()
Pool-->>DC: connection
deactivate DC
App->>DB: execute query (e.g., SELECT 1)
App->>DC: release_connection(conn)
activate DC
DC->>Pool: release(conn)
deactivate DC
Note over App,DC: Shutdown / teardown
App->>DC: close_db_connection()
activate DC
DC->>DC: disconnect() [classmethod]
DC->>Pool: close()
Pool->>DB: close connections
deactivate DC
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
.env (1)
5-18: DB_ and TEST_DB_ entries look fine; consider minor .env linting cleanup**The new PostgreSQL config keys are clear and match the connection code. Two small, optional tweaks:
- If you want
dotenv-linterclean, drop the quotes on values without spaces and order the keys as it suggests (e.g.,DB_HOST,DB_PORT,DB_USER,DB_PASSWORD,DB_NAMEand similarly for TEST_*).- Keep this file strictly to placeholders/non-sensitive values so real credentials are not committed; rely on environment overrides in CI/production.
backend/app/db/tests/test_connection.py (2)
22-40: Async pool and connection tests are sound; add a custom pytest marker for selective integration-test execution.The tests correctly exercise pool creation, connection acquisition/release, and a simple
SELECT 1query. These are integration tests requiring a live PostgreSQL instance with TEST_DB_* credentials and database.To support fast unit-test-only runs in CI and local development, register a custom pytest marker in
pytest.iniand decorate these tests:[pytest] markers = db: marks tests that require a live Postgres databaseThen add
@pytest.mark.dbto the test functions (alongside@pytest.mark.asyncio). This lets you runpytest -m "not db"for unit tests only orpytest -m "db"for integration tests.If tests will run in parallel, ensure each uses transaction-scoped connections with explicit rollback to prevent test pollution.
41-46: Test private implementation details indirectly; avoid test-ordering dependenciesThe test asserts on
DatabaseConnection._pooland callsconnect()to restore state for subsequent tests. Per asyncpg best practices, avoid asserting on private attributes; instead:
- Test observable behavior via public API: after
disconnect(), verify thatget_connection()oracquire()either fails or automatically recreates the pool (depending on your design intent).- Remove the in-test reconnect; use a fixture to reset state between tests, or design
disconnect()to reset only what is necessary without coupling downstream tests.backend/app/db/connection.py (1)
2-7: Prefer loading .env at application startup, not inside the DB moduleCalling
load_dotenv()at import time makes this module have side effects and couples it to a specific configuration strategy. Best practice is to callload_dotenv()as early as possible in your application entrypoint (top of main/ASGI module or app/init.py) before any module that reads os.environ is imported.For FastAPI projects, Pydantic's built-in
env_filesupport via BaseSettings/pydantic_settings is often cleaner, as Pydantic handles loading .env automatically when the Settings object is instantiated.This keeps configuration centralized and avoids side effects during library import.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env(1 hunks)backend/app/db/connection.py(1 hunks)backend/app/db/tests/test_connection.py(1 hunks)requirements.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/app/db/tests/test_connection.py (1)
backend/app/db/connection.py (7)
DatabaseConnection(9-48)initialize_db_connection(51-52)close_db_connection(54-55)connect(13-29)get_connection(40-43)release_connection(46-48)disconnect(32-37)
🪛 dotenv-linter (4.0.0)
.env
[warning] 7-7: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 8-8: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 8-8: [UnorderedKey] The DB_PASSWORD key should go before the DB_USER key
(UnorderedKey)
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 9-9: [UnorderedKey] The DB_HOST key should go before the DB_PASSWORD key
(UnorderedKey)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [UnorderedKey] The DB_PORT key should go before the DB_USER key
(UnorderedKey)
[warning] 11-11: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 11-11: [UnorderedKey] The DB_NAME key should go before the DB_PASSWORD key
(UnorderedKey)
[warning] 14-14: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 15-15: [UnorderedKey] The TEST_DB_PASSWORD key should go before the TEST_DB_USER key
(UnorderedKey)
[warning] 16-16: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 16-16: [UnorderedKey] The TEST_DB_HOST key should go before the TEST_DB_PASSWORD key
(UnorderedKey)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [UnorderedKey] The TEST_DB_PORT key should go before the TEST_DB_USER key
(UnorderedKey)
[warning] 18-18: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 18-18: [UnorderedKey] The TEST_DB_NAME key should go before the TEST_DB_PASSWORD key
(UnorderedKey)
🔇 Additional comments (1)
requirements.txt (1)
10-11: Verify asyncpg 0.28.0 compatibility in your deployment environment; project lacks explicit version constraintsasyncpg 0.28.0 supports Python 3.7+ and is tested against PostgreSQL 9.5–15. This covers a broad range, but your project does not declare explicit Python or PostgreSQL versions anywhere (no Dockerfile, docker-compose, setup.py, or .python-version).
Before merging, confirm:
- Your deployment image (Docker/Kubernetes) specifies Python and PostgreSQL versions that fall within asyncpg 0.28.0's tested range
- Test environment versions match production
- FastAPI 0.121.3 and other dependencies work with those same versions
Consider adding version constraints to your infrastructure-as-code files (Dockerfile, docker-compose.yml) to ensure consistency across development, testing, and production.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
backend/app/db/connection.py (2)
29-30: Consider making pool sizes configurable.Hardcoded pool sizes (
min_size=1,max_size=10) may not suit all deployment environments. Consider reading these from environment variables with sensible defaults for production flexibility.Example implementation:
- min_size=1, - max_size=10, + min_size=int(os.getenv("DB_POOL_MIN_SIZE", "1")), + max_size=int(os.getenv("DB_POOL_MAX_SIZE", "10")),
38-44: Consider using lock in disconnect for edge-case safety.While
disconnectis typically called once during shutdown, using_pool_lockwould prevent potential race conditions if multiple tasks attempt concurrent disconnection.Apply this diff:
@classmethod async def disconnect(cls): - if cls._pool: - logger.info("Closing database connection pool.") - await cls._pool.close() - cls._pool = None - logger.info("Database connection pool closed.") + async with cls._pool_lock: + if cls._pool: + logger.info("Closing database connection pool.") + await cls._pool.close() + cls._pool = None + logger.info("Database connection pool closed.")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/app/db/__pycache__/connection.cpython-313.pycis excluded by!**/*.pycbackend/app/db/tests/__pycache__/test_connection.cpython-313-pytest-8.2.0.pycis excluded by!**/*.pyc
📒 Files selected for processing (4)
backend/app/db/connection.py(1 hunks)backend/app/db/tests/test_connection.py(1 hunks)pytest.ini(1 hunks)requirements.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/db/tests/test_connection.py
- requirements.txt
🧰 Additional context used
🪛 Ruff (0.14.5)
backend/app/db/connection.py
33-33: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🔇 Additional comments (6)
pytest.ini (1)
1-4: LGTM! Pytest configuration is appropriate for async database tests.The configuration correctly sets up pytest-asyncio with auto mode and restricts test discovery to the database tests directory. The pythonpath setting ensures proper module resolution.
backend/app/db/connection.py (5)
1-9: LGTM! Imports and environment loading follow standard patterns.The module-level
load_dotenv()call is appropriate for loading database credentials when the module is imported.
11-12: LGTM! Proper structured logging setup.The use of
logging.getLogger(__name__)follows Python best practices and addresses previous review feedback about replacing print statements.
13-15: LGTM! Singleton pattern with proper concurrency control.The double-checked locking pattern using
asyncio.Lock()correctly addresses previous review feedback about race conditions during pool initialization.
52-55: LGTM! Safe connection release implementation.The method properly checks for pool and connection existence before releasing.
58-62: LGTM! Clean module-level API.The helper functions provide a clean, simple interface for database lifecycle management.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
backend/app/db/connection.py (4)
8-11: Optional: avoid loading.envat import time inside a low-level DB module
load_dotenv()and logger setup here are fine for a self-contained service, but they couple this utility to a specific configuration mechanism. If this module is ever reused (e.g., different entrypoints/tests) you may prefer to:
- Call
load_dotenv()once in your main app startup / test bootstrap, and- Keep this module focused purely on reading
os.environ.This is just an ergonomics/architecture tweak, not a correctness issue.
23-47: Env var & port validation look solid; lints about long error messages are purely stylisticThe explicit extraction of
DB_*vars, clearmissing_varscheck, andDB_PORTinteger coercion with a chainedValueErrorgive good, early‑failing diagnostics. Functionally this is in good shape.Ruff’s TRY003/TRY301 hints here are stylistic; you can safely ignore them, or, if you want to appease the linter, factor validation into a small helper (e.g.,
_load_db_config()) that raises the sameValueErrors.
17-21: Singleton pool initialization with double‑checked lock looks correct; consider configurable pool sizingThe lazy, double‑checked initialization under
_pool_lockis concurrency‑safe and avoids multiple pools. Logging on success/failure is also appropriate and doesn’t leak secrets.One improvement you might want later: make
min_size/max_size(and possibly other pool settings) configurable via env or settings, so test, staging, and prod can tune pool size without code changes.Also applies to: 49-62
72-82: Consider an async context manager to pairget_connection/release_connectionsafely
get_connectionwith a 30‑second acquire timeout plusrelease_connectionis fine, but call sites must remember to always release, typically withtry/finally.To reduce leak risk and simplify usage, you could add an async context manager wrapper, e.g.:
from contextlib import asynccontextmanager @asynccontextmanager async def db_connection(): conn = await DatabaseConnection.get_connection() try: yield conn finally: await DatabaseConnection.release_connection(conn)Callers can then use
async with db_connection() as conn:and can’t accidentally forget to release.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/app/db/__pycache__/connection.cpython-313.pycis excluded by!**/*.pycbackend/app/db/tests/__pycache__/test_connection.cpython-313-pytest-8.2.0.pycis excluded by!**/*.pyc
📒 Files selected for processing (1)
backend/app/db/connection.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.5)
backend/app/db/connection.py
40-42: Abstract raise to an inner function
(TRY301)
40-42: Avoid specifying long messages outside the exception class
(TRY003)
47-47: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
backend/app/db/connection.py (2)
64-71: Pool shutdown logic is straightforward and correctClosing the pool once, resetting
_pooltoNone, and logging before/after is a clean, idempotent shutdown flow for typical startup/shutdown hooks. No functional issues here.
84-88: Helper lifecycle functions align well with framework startup/shutdown hooks
initialize_db_connectionandclose_db_connectionare thin and clear wrappers around the class API, which will plug neatly into FastAPI/Starlette lifespan events or similar. No changes needed.
|
sweet, async PostgreSQL connection will boost performance—merging this now. |
Overview: This PR introduces an asynchronous database connection module for PostgreSQL.
Changes
.envusing environment variables.backend/app/db/connection.py.Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.