fix(config): accept host:port in POSTGRES_HOST (pooler support)#393
Merged
Conversation
Services that talk to PostgreSQL crashed on startup when POSTGRES_HOST
carried an embedded port (e.g. a PgBouncer pooler configured as
"pgbouncer:6432"):
invalid literal for int() with base 10: '6432:5432'
Root cause: `_build_postgres_connstr()` unconditionally appended `:5432`
to POSTGRES_HOST, producing "pgbouncer:6432:5432", which downstream
host/port splitters then `int()`-ed and threw on.
Fix: add a single shared `parse_postgres_host_port()` helper in
common/config.py that robustly splits host/port — an embedded port wins,
otherwise POSTGRES_PORT (default 5432) is used; the two are never
concatenated. IPv6-bracketed hosts are handled defensively. Route every
PostgreSQL consumer through it (tableinator, brainztableinator, dashboard,
insights, api, api/setup, api/admin_setup, schema-init), replacing the
inconsistent ad-hoc split/rsplit logic.
Add unit tests for the parser and connstr builder, and document that
POSTGRES_HOST may include a port in docs/configuration.md and .env.example.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CmjnLX2V3bSwz1F9jvYQUY
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
E2E Coverage (chromium)
|
Contributor
E2E Coverage (webkit)
|
Contributor
E2E Coverage (firefox)
|
Contributor
E2E Coverage (webkit - iPhone 15)
|
Contributor
E2E Coverage (webkit - iPad Pro 11)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In the homelab deployment, every service that talks to PostgreSQL now connects through a PgBouncer pooler configured via
POSTGRES_HOST="pgbouncer:6432"(host and port combined; no separatePOSTGRES_PORT). On startup the services crashed with:Observed in tableinator and brainztableinator, but it affected every PostgreSQL consumer via the shared
common.configpath.Root cause
_build_postgres_connstr()incommon/config.pyunconditionally appended:5432toPOSTGRES_HOST:Downstream host/port splitters then
int()-ed the bogus"6432:5432"and threw. The various consumers also used inconsistent parsing (split(":", 1)vsrsplit(":", 1)), so the result was wrong even without a crash.Fix
parse_postgres_host_port(value, default_port=5432) -> (host, port)helper incommon/config.py:POSTGRES_PORT(default5432). The two ports are never concatenated.[::1]:6432, bare::1), empty/None, malformed ports, and whitespace._build_postgres_connstr()to use the helper and emit a canonical, round-trippablehost:port(IPv6 bracketed).tableinator,brainztableinator,dashboard,insights,api,api/setup,api/admin_setup,schema-init— replacing the ad-hoc split/rsplit logic (fix-one-fix-all).POSTGRES_HOSTmay include a port indocs/configuration.md(incl. a newPOSTGRES_PORTrow) and.env.example.Tests
TestParsePostgresHostPortandTestBuildPostgresConnstrclasses intests/common/test_config.pycovering:pgbouncer:6432→(pgbouncer, 6432); bare host +POSTGRES_PORT→(host, 5432);postgres:5432with noPOSTGRES_PORT→(postgres, 5432); malformed/empty cases; IPv6; and the "never concatenate" regression.tests/common,insights,schema-init,dashboard,api,tableinator,brainztableinator).Verification
Reproduced the previously-crashing startup path against
POSTGRES_HOST="pgbouncer:6432":Also confirmed
schema-init,api/setup, andapi/admin_setupbuild correct connection params (incl.POSTGRES_PORTfallback).🤖 Generated with Claude Code