Repository navigation
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| """Initialize database connection pool""" | ||
| try: | ||
| # Create async engine with connection pooling | ||
| database_url = f"postgresql+asyncpg://{settings.supabase_db_user}:{settings.supabase_db_password}@{settings.supabase_db_host}:{settings.supabase_db_port}/{settings.supabase_db_name}" | ||
| database_url = settings.database_url | ||
|
|
||
| self.engine = create_async_engine( | ||
| database_url, | ||
| poolclass=QueuePool, | ||
| # poolclass=QueuePool, | ||
| pool_size=20, # Number of connections to maintain | ||
| max_overflow=30, # Additional connections when needed | ||
| pool_pre_ping=True, # Validate connections |
There was a problem hiding this comment.
Bug: DATABASE_URL in docker-compose.yml lacks the +asyncpg driver scheme, causing create_async_engine in DatabasePool.initialize() to fail and leave the connection pool uninitialized.
Severity: HIGH
Suggested Fix
Update DATABASE_URL in docker-compose.yml to use postgresql+asyncpg://, or normalize the driver scheme inside DatabasePool.initialize() / config.py to ensure +asyncpg is always used for create_async_engine.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/app/core/database_pool.py#L15-L25
Potential issue: `DatabasePool.initialize()` uses `settings.database_url` with
`create_async_engine`. In `docker-compose.yml`, `DATABASE_URL` is configured using
`postgresql://` without the `+asyncpg` dialect prefix, overriding the default setting in
`config.py`. As a result, `create_async_engine` fails due to an incompatible sync
driver. The exception is swallowed in `initialize()`, setting `engine` and
`session_factory` to `None` while startup reports success, causing database-dependent
calls like `calculate_total_revenue` to fall back to mock data.
Did we get this right? 👍 / 👎 to inform future reviews.
| await db_pool.initialize() | ||
|
|
||
| if db_pool.session_factory: | ||
| async with db_pool.get_session() as session: | ||
| async with (await db_pool.get_session()) as session: | ||
| # Use SQLAlchemy text for raw SQL | ||
| from sqlalchemy import text | ||
|
|
There was a problem hiding this comment.
Bug: calculate_total_revenue instantiates and initializes a new DatabasePool on every call without disposing of the engine, leading to potential database connection leaks.
Severity: MEDIUM
Suggested Fix
Inject or import the global db_pool instance initialized in main.py instead of creating and initializing a new DatabasePool() instance inside calculate_total_revenue.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: backend/app/services/reservations.py#L44-L50
Potential issue: `calculate_total_revenue` instantiates and calls `initialize()` on a
new `DatabasePool` instance during every execution rather than reusing a shared
instance. Each call creates a new `AsyncEngine` with connection pooling. The function
never disposes of the engine or calls `db_pool.close()`, holding open database
connections until garbage collection runs and risking database connection exhaustion.
Did we get this right? 👍 / 👎 to inform future reviews.
Instead of supabase legacy db connection pool, started using db pool
When writing to cache and reading from cache, added a secondary part to cache key (tenant id)