Skip to content

Fixing database connection and Tenant separation errors #236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
malkoch wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fixing database connection and Tenant separation errors #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
207c502
change supabase pool to db pool
malkoch Nov 9, 2026
a8a4d7c
fix db pool url connection and aysnc context manager
malkoch Nov 9, 2026
0f9b277
use asyncpg as connection string
malkoch Nov 9, 2026
837cbc6
updating the requirements
malkoch Nov 9, 2026
753c025
apply context manager changes
malkoch Nov 9, 2026
7757d32
adding tenant id in cache key
malkoch Nov 9, 2026
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
2 changes: 1 addition & 1 deletion backend/app/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

class Settings(BaseSettings):
# Core settings
database_url: str = "postgresql://postgres:postgres@db:5432/propertyflow"
database_url: str = "postgresql+asyncpg://postgres:postgres@db:5432/propertyflow"
redis_url: str = "redis://redis:6379/0"
secret_key: str = "debug_challenge_secret"

Expand Down
8 changes: 4 additions & 4 deletions backend/app/core/database_pool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ async def initialize(self):
"""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
Comment on lines 15 to 25

@sentry sentry Bot Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Expand Down Expand Up @@ -56,5 +56,5 @@ async def get_session(self) -> AsyncSession:

async def get_db_session() -> AsyncSession:
"""Dependency to get database session"""
async with db_pool.get_session() as session:
async with (await db_pool.get_session()) as session:
yield session
8 changes: 4 additions & 4 deletions backend/app/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@ async def lifespan(app: FastAPI):

# Initialize Supabase connection pool
try:
from .core.supabase_connection_pool import supabase_pool
from .core.database_pool import db_pool

await supabase_pool.initialize()
logger.info("✅ Supabase connection pool initialized")
await db_pool.initialize()
logger.info("✅ Database connection pool initialized")
except Exception as e:
logger.error(f"❌ Supabase connection pool initialization failed: {e}")
logger.error(f"❌ Database connection pool initialization failed: {e}")
# Continue startup - fallback to direct connections

# Initialize Redis connection with timeout
Expand Down
2 changes: 1 addition & 1 deletion backend/app/services/cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ async def get_revenue_summary(property_id: str, tenant_id: str) -> Dict[str, Any
"""
Fetches revenue summary, utilizing caching to improve performance.
"""
cache_key = f"revenue:{property_id}"
cache_key = f"revenue:{property_id}:{tenant_id}"

# Try to get from cache
cached = await redis_client.get(cache_key)
Expand Down
2 changes: 1 addition & 1 deletion backend/app/services/reservations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async def calculate_total_revenue(property_id: str, tenant_id: str) -> Dict[str,
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

Comment on lines 44 to 50

@sentry sentry Bot Nov 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Expand Down
3 changes: 2 additions & 1 deletion backend/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ lz4
orjson
Pillow==10.4.0
psycopg2-binary
psycopg
pydantic-settings
pydantic[email]
pyhumps==3.8.0
Expand All @@ -18,7 +19,7 @@ pytz
redis>=5.0.0
requests
sendgrid
sqlalchemy>=2.0.0
sqlalchemy[asyncio]>=2.0.0
supabase
tenacity>=8.0.0
uvicorn[standard]
Expand Down