fix: use Docker service name 'postgres' as default DB host in alembic.ini - #1705
fix: use Docker service name 'postgres' as default DB host in alembic.ini#1705esazonova wants to merge 1 commit into
Conversation
….ini Change the default sqlalchemy.url in alembic.ini from localhost to postgres (the Docker service name). This ensures the database-migration container can connect to PostgreSQL when running in Docker Compose, even if the DBHOST environment variable is not explicitly set. Fixes genlayerlabs#1698
|
This PR targeted I retargeted it to |
📝 WalkthroughWalkthroughAlembic’s default PostgreSQL URL now targets the Docker ChangesDatabase migration configuration
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/database_handler/alembic.ini`:
- Around line 63-65: Update the URL selection in env.py so the runtime
DB_URL/DBHOST configuration is used when provided, but the Alembic
sqlalchemy.url value is preserved when runtime configuration is absent; ensure
fresh Compose startup resolves the postgres service host instead of defaulting
to localhost.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9927bd4b-4b33-489c-8ecb-d150c04d9af7
📒 Files selected for processing (1)
backend/database_handler/alembic.ini
| # Default URL uses Docker service name 'postgres' as host. | ||
| # Overridden at runtime by env.py using DBHOST, DBUSER, DBPASSWORD, DBPORT, DBNAME env vars. | ||
| sqlalchemy.url = postgresql://postgres:postgres@postgres:5432/genlayer_state |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The new default URL is bypassed by env.py.
backend/database_handler/migration/env.py always assigns url from DB_URL; when DBHOST is unset, that code defaults to localhost instead of falling back to this Alembic setting. Therefore, changing this line to postgres does not fix the fresh genlayer up path. Update env.py to use the INI URL when runtime configuration is absent, or ensure Compose supplies DBHOST=postgres by default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/database_handler/alembic.ini` around lines 63 - 65, Update the URL
selection in env.py so the runtime DB_URL/DBHOST configuration is used when
provided, but the Alembic sqlalchemy.url value is preserved when runtime
configuration is absent; ensure fresh Compose startup resolves the postgres
service host instead of defaulting to localhost.
Summary
This PR fixes the database migration container failing on fresh
genlayer upby updating the defaultsqlalchemy.urlinalembic.inito use the Docker service namepostgresinstead oflocalhost.Problem
The
database-migrationcontainer was failing with:This happened because
alembic.inihardcodedlocalhostas the database host, but in Docker Compose the PostgreSQL service is accessible via the service namepostgres, notlocalhost.Solution
Changed the default
sqlalchemy.urlinalembic.inifrom:to:
This matches the Docker Compose service name and the default
DBHOSTvalue in.env.example.Testing
env.pystill properly overrides this URL with environment variablesDBHOSTis not setFixes #1698
Summary by CodeRabbit