Repository navigation
feat(OMN-2065): Replace POSTGRES_DATABASE with per-service DB URLs - #290
Merged
jonahgabriel merged 55 commits intoFeb 11, 2026
Merged
jonahgabriel merged 55 commits into
jonahgabriel merged 55 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSwitches PostgreSQL configuration from multiple POSTGRES_* environment variables to per-service DSN environment variables (primarily OMNIBASE_INFRA_DB_URL), updates Docker, runtime models, CLI/plugins, scripts, tests, and docs to read/parse DSNs, and standardizes the default database name to omnibase_infra. Changes
Sequence Diagram(s)sequenceDiagram
actor Env as Environment
participant CLI as CLI / Script
participant Runtime as Runtime / Plugin
participant Pool as Postgres Pool (asyncpg)
participant DB as PostgreSQL
Env->>CLI: OMNIBASE_INFRA_DB_URL (or legacy POSTGRES_*)
CLI->>Runtime: pass DSN
Runtime->>Runtime: validate DSN (scheme, database)
Runtime->>Pool: asyncpg.create_pool(dsn)
Pool->>DB: connect using DSN
DB-->>Pool: connection established
Pool-->>Runtime: pool ready
Runtime-->>CLI: success / error (contextual)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/integration/registration/e2e/conftest.py (1)
277-323:⚠️ Potential issue | 🟠 MajorFallback DSN still targets
omninode_bridge.
With the E2E stack now usingomnibase_infra, the fallback path will point to a non-existent DB when OMNIBASE_INFRA_DB_URL is unset.Proposed fix
return ( f"postgresql://{encoded_user}:{encoded_password}" - f"@{POSTGRES_HOST}:{POSTGRES_PORT}/omninode_bridge" + f"@{POSTGRES_HOST}:{POSTGRES_PORT}/omnibase_infra" )src/omnibase_infra/nodes/node_registration_orchestrator/plugin.py (1)
224-262:⚠️ Potential issue | 🟡 MinorTreat whitespace-only OMNIBASE_INFRA_DB_URL as unset.
A whitespace value currently passes the truthiness check, which triggers activation and a less-clear pool error. Stripping provides a clean fail-fast message.🛠️ Suggested fix
- db_url = os.getenv("OMNIBASE_INFRA_DB_URL") - if not db_url: + db_url = (os.getenv("OMNIBASE_INFRA_DB_URL") or "").strip() + if not db_url: logger.debug( "Registration plugin inactive: OMNIBASE_INFRA_DB_URL not set " "(correlation_id=%s)", config.correlation_id, ) return False @@ - postgres_dsn = os.getenv("OMNIBASE_INFRA_DB_URL", "") - if not postgres_dsn: + postgres_dsn = (os.getenv("OMNIBASE_INFRA_DB_URL") or "").strip() + if not postgres_dsn: context = ModelInfraErrorContext.with_correlation( correlation_id=correlation_id, transport_type=EnumInfraTransportType.DATABASE, operation="create_postgres_pool", )scripts/dlq_replay.py (1)
511-543:⚠️ Potential issue | 🟠 MajorFail fast when tracking is enabled but DSN is missing.
--enable-trackingcurrently degrades silently (tracking stays off). That contradicts the “required” docs and makes failures harder to diagnose.🛠️ Suggested fix
def build_tracking_dsn(self) -> str | None: """Return PostgreSQL DSN for replay tracking. @@ if not self.enable_tracking: return None - if not self.postgres_dsn: - return None - return self.postgres_dsn + if not self.postgres_dsn: + raise ValueError( + "OMNIBASE_INFRA_DB_URL is required when --enable-tracking is set." + ) + return self.postgres_dsn
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 35-60: Reorder the per-service DB URL env keys to satisfy
dotenv-linter ordering: move OMNICLAUDE_DB_URL and OMNIDASH_ANALYTICS_DB_URL so
they appear before OMNIINTELLIGENCE_DB_URL; keep the other entries
(OMNIBASE_INFRA_DB_URL, OMNIMEMORY_DB_URL, OMNINODE_CLOUD_DB_URL, etc.)
unchanged and preserve their values/format.
In `@src/omnibase_infra/cli/commands.py`:
- Around line 205-218: The _get_db_dsn function raises a click.ClickException
without including a correlation_id; update _get_db_dsn to accept an optional
correlation_id parameter (or generate one with uuid.uuid4() if None) and include
that correlation_id in the ClickException message so errors are traceable;
callers that call _get_db_dsn (search for _get_db_dsn invocations) should be
updated to pass along the incoming correlation_id when available, and the
ClickException text produced by _get_db_dsn should embed the correlation_id
value for consistent error context.
In `@src/omnibase_infra/runtime/models/model_postgres_pool_config.py`:
- Around line 104-123: The error message currently echoes the raw DSN (dsn!r)
which may leak credentials; update the DSN-parsing block (the urlparse/dsn
handling in the classmethod that returns cls) to avoid including the full DSN in
exceptions—use safe fields from parsed (e.g., database, parsed.hostname,
parsed.port) or a sanitized DSN (no username/password) in the ValueError
messages instead of dsn!r; specifically change the second error message that
raises ValueError(msg) to reference only non-sensitive info like the parsed
database name or host/port (use variables database, parsed.hostname,
parsed.port) and remove any inclusion of the original dsn string.
In `@tests/helpers/util_postgres.py`:
- Around line 249-260: The code currently defaults database to "omninode_bridge"
when parsing a DSN (db_url_var) which can hide malformed DSNs; change the DB
extraction in the db_url branch so database is left empty (e.g. "" or None) when
parsed.path is missing instead of defaulting to "omninode_bridge", so that
cls(..., database=...) will make is_configured false and cause tests to skip;
update the return in the db_url handling (the call that constructs cls with
host, port, database, user, password) to use the parsed database as-is without
the fallback default, keeping the other fallbacks (parsed.hostname ->
"localhost", parsed.port -> DEFAULT_POSTGRES_PORT, parsed.username ->
default_user) unchanged.
In `@tests/integration/dlq/conftest.py`:
- Around line 21-23: Update the OMNIBASE_INFRA_DB_URL example value to match the
new per-service database naming (replace the old "omninode_bridge" DB name in
the example DSN with the new service-specific database name), i.e. edit the
example postgresql://.../omninode_bridge to use the new database identifier so
the OMNIBASE_INFRA_DB_URL example is accurate and consistent with the
per-service DB URL convention.
In `@tests/integration/handlers/test_registration_storage_handler_swapping.py`:
- Around line 441-469: The test skip condition does not consider
OMNIBASE_INFRA_DB_URL so tests are skipped even when a DSN is provided; update
the POSTGRES_AVAILABLE logic (used in
tests/integration/handlers/test_registration_storage_handler_swapping.py) to
treat a non-empty OMNIBASE_INFRA_DB_URL as available — e.g., set
POSTGRES_AVAILABLE = bool(os.getenv("OMNIBASE_INFRA_DB_URL")) or combine it with
the existing host/password check so the test harness recognizes either the DSN
or the individual POSTGRES_* vars as valid.
In `@tests/integration/ledger/conftest.py`:
- Around line 36-58: The fallback DSN in _get_postgres_dsn currently hardcodes
the legacy database "/omninode_bridge"; change that hardcoded DB name to the new
infra DB name used everywhere else (i.e., make the fallback use the same DB name
as the OMNIBASE_INFRA_DB_URL default) so the fallback won’t write to the legacy
DB, and update the test skip message (where pytest.skip is called) to reference
OMNIBASE_INFRA_DB_URL in addition to POSTGRES_HOST/POSTGRES_PASSWORD so users
see the correct configuration variables to set.
In `@tests/integration/runtime/test_projector_shell_database.py`:
- Around line 17-28: The examples for OMNIBASE_INFRA_DB_URL are inconsistent:
the main DSN example uses the database name "/postgres" while the remote example
uses "/omninode_bridge"; update one of them so both examples use the same DB
name (e.g., change the remote example to end with "/postgres" or change the main
example to "/omninode_bridge") to keep OMNIBASE_INFRA_DB_URL, POSTGRES_* docs
consistent across the file.
In `@tests/integration/services/snapshot/test_store_postgres_integration.py`:
- Around line 71-83: Replace the legacy POSTGRES_* fallback and hardcoded
"omninode_bridge" DSN by requiring OMNIBASE_INFRA_DB_URL only: if
os.getenv("OMNIBASE_INFRA_DB_URL") is set return it, otherwise return None (or
raise) immediately; remove the host/port/user/password logic and update any test
skip reason/docstring to state that OMNIBASE_INFRA_DB_URL is required. Ensure
references in the code use OMNIBASE_INFRA_DB_URL (not
POSTGRES_HOST/PORT/USER/PASSWORD) so the integration test fails fast when the
DSN is missing.
🧹 Nitpick comments (4)
docker/docker-compose.infra.yml (1)
463-466: Prefer reusing OMNIBASE_INFRA_DB_URL for agent-actions-consumer.
Keeps a single DSN source-of-truth and avoids drift.Proposed refactor
- OMNIBASE_INFRA_AGENT_ACTIONS_POSTGRES_DSN: postgresql://${POSTGRES_USER:-postgres}:${POSTGRES_PASSWORD}@${POSTGRES_HOST:-postgres}:${POSTGRES_PORT:-5432}/omnibase_infra + OMNIBASE_INFRA_AGENT_ACTIONS_POSTGRES_DSN: ${OMNIBASE_INFRA_DB_URL:-postgresql://postgres:${POSTGRES_PASSWORD}@postgres:5432/omnibase_infra}tests/integration/handlers/test_registration_storage_handler_swapping.py (1)
672-700: Consider deduplicating DSN parsing logic.
The same URL parsing defaults appear here and increate_handler_by_type. Extracting a small helper will prevent drift and keep defaults consistent.tests/integration/handlers/conftest.py (2)
64-72: Documentation inconsistency: POSTGRES_HOST/PASSWORD listed as "required" but are now fallback.Lines 64-66 state
POSTGRES_HOSTandPOSTGRES_PASSWORDare "(required)", but with the new changes they're only required whenOMNIBASE_INFRA_DB_URLis not set. Consider restructuring the documentation to match the pattern used in other conftest files:📝 Suggested documentation fix
Database Handlers ================= -Environment Variables (required): - POSTGRES_HOST: PostgreSQL hostname (required) - POSTGRES_PASSWORD: Database password (required) Environment Variables (preferred): OMNIBASE_INFRA_DB_URL: Full PostgreSQL DSN (preferred, overrides individual vars) Environment Variables (fallback): + POSTGRES_HOST: PostgreSQL hostname (required if DSN not set) + POSTGRES_PASSWORD: Database password (required if DSN not set) POSTGRES_PORT: PostgreSQL port (default: 5432) POSTGRES_USER: Database username (default: postgres)
217-220: Consider adding whitespace validation for_OMNIBASE_INFRA_DB_URL.The code at lines 204-215 validates
POSTGRES_PASSWORDagainst empty/whitespace-only values with.strip(). However,_OMNIBASE_INFRA_DB_URLonly usesbool()which would consider whitespace-only strings like" "as truthy, potentially causing connection failures later.For consistency with the defensive checks applied to
POSTGRES_PASSWORD:🛡️ Suggested defensive check
# Primary: OMNIBASE_INFRA_DB_URL (full DSN). Fallback: individual POSTGRES_* vars. -_OMNIBASE_INFRA_DB_URL = os.getenv("OMNIBASE_INFRA_DB_URL") +_OMNIBASE_INFRA_DB_URL = os.getenv("OMNIBASE_INFRA_DB_URL") +# Normalize whitespace-only values to None +if _OMNIBASE_INFRA_DB_URL and not _OMNIBASE_INFRA_DB_URL.strip(): + _OMNIBASE_INFRA_DB_URL = None
Comment on lines
35
to
+60
| # ============================================================================= | ||
| # PostgreSQL Database Configuration | ||
| # Per-Service Database URLs | ||
| # ============================================================================= | ||
| # Each service connects to its own database using a full DSN (connection URL). | ||
| # Format: postgresql://role:password@host:port/database | ||
| # | ||
| # SECURITY: Replace __REPLACE_WITH_SECURE_PASSWORD__ with a secure password. | ||
| # Generate using: openssl rand -hex 32 | ||
| # NOTE: Use -hex (not -base64) because base64 output includes '/' and '+' | ||
| # which break URI parsing. | ||
| # | ||
| # For Docker/integration tests: Use hostname (resolves via extra_hosts) | ||
| # POSTGRES_HOST=omninode-bridge-postgres | ||
| # e.g., postgresql://role:pass@omninode-bridge-postgres:5432/omnibase_infra | ||
| # For local scripts/direct access: Use IP address | ||
| # POSTGRES_HOST=<your-server-ip> | ||
| POSTGRES_HOST=your-postgres-host | ||
| POSTGRES_PORT=5432 | ||
| POSTGRES_DATABASE=your_database | ||
| POSTGRES_USER=postgres | ||
| # SECURITY: Replace this placeholder with a secure password | ||
| # Generate using: openssl rand -base64 32 | ||
| POSTGRES_PASSWORD=__REPLACE_WITH_SECURE_PASSWORD__ | ||
| # e.g., postgresql://role:pass@<your-server-ip>:5432/omnibase_infra | ||
| OMNIBASE_INFRA_DB_URL=postgresql://role_omnibase_infra:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnibase_infra | ||
| OMNIINTELLIGENCE_DB_URL=postgresql://role_omniintelligence:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omniintelligence | ||
| OMNICLAUDE_DB_URL=postgresql://role_omniclaude:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omniclaude | ||
| OMNIMEMORY_DB_URL=postgresql://role_omnimemory:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnimemory | ||
| OMNINODE_CLOUD_DB_URL=postgresql://role_omninode_cloud:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omninode_cloud | ||
| OMNIDASH_ANALYTICS_DB_URL=postgresql://role_omnidash:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnidash_analytics | ||
|
|
||
| # PostgreSQL advanced settings (optional - defaults shown) | ||
| # Connection pooling configuration for high-performance scenarios | ||
| # ============================================================================= | ||
| # PostgreSQL Pool Configuration (Optional) | ||
| # ============================================================================= | ||
| # Connection pooling configuration for high-performance scenarios. |
There was a problem hiding this comment.
Fix dotenv-linter key ordering in the per-service DB URL block.
The linter expects OMNICLAUDE_DB_URL and OMNIDASH_ANALYTICS_DB_URL to appear before OMNIINTELLIGENCE_DB_URL.
Proposed fix
OMNIBASE_INFRA_DB_URL=postgresql://role_omnibase_infra:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnibase_infra
-OMNIINTELLIGENCE_DB_URL=postgresql://role_omniintelligence:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omniintelligence
OMNICLAUDE_DB_URL=postgresql://role_omniclaude:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omniclaude
-OMNIMEMORY_DB_URL=postgresql://role_omnimemory:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnimemory
-OMNINODE_CLOUD_DB_URL=postgresql://role_omninode_cloud:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omninode_cloud
OMNIDASH_ANALYTICS_DB_URL=postgresql://role_omnidash:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnidash_analytics
+OMNIINTELLIGENCE_DB_URL=postgresql://role_omniintelligence:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omniintelligence
+OMNIMEMORY_DB_URL=postgresql://role_omnimemory:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omnimemory
+OMNINODE_CLOUD_DB_URL=postgresql://role_omninode_cloud:__REPLACE_WITH_SECURE_PASSWORD__@your-postgres-host:5432/omninode_cloud🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 52-52: [UnorderedKey] The OMNICLAUDE_DB_URL key should go before the OMNIINTELLIGENCE_DB_URL key
(UnorderedKey)
[warning] 55-55: [UnorderedKey] The OMNIDASH_ANALYTICS_DB_URL key should go before the OMNIINTELLIGENCE_DB_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env.example around lines 35 - 60, Reorder the per-service DB URL env keys
to satisfy dotenv-linter ordering: move OMNICLAUDE_DB_URL and
OMNIDASH_ANALYTICS_DB_URL so they appear before OMNIINTELLIGENCE_DB_URL; keep
the other entries (OMNIBASE_INFRA_DB_URL, OMNIMEMORY_DB_URL,
OMNINODE_CLOUD_DB_URL, etc.) unchanged and preserve their values/format.
Comment on lines
+71
to
+83
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | ||
| if db_url: | ||
| return db_url | ||
|
|
||
| host = os.getenv("POSTGRES_HOST") | ||
| port = os.getenv("POSTGRES_PORT", "5432") | ||
| database = os.getenv("POSTGRES_DATABASE", "omninode_bridge") | ||
| user = os.getenv("POSTGRES_USER", "postgres") | ||
| password = os.getenv("POSTGRES_PASSWORD") | ||
|
|
||
| if not host or not password: | ||
| return None | ||
|
|
||
| return f"postgresql://{user}:{password}@{host}:{port}/{database}" | ||
| return f"postgresql://{user}:{password}@{host}:{port}/omninode_bridge" |
There was a problem hiding this comment.
Avoid legacy POSTGRES_ fallback in integration tests.*
The helper still builds a DSN from POSTGRES_* and defaults to omninode_bridge, which can let CI pass without validating the new OMNIBASE_INFRA_DB_URL requirement. If the intent is fail-fast on missing DSN, return None (or raise) when OMNIBASE_INFRA_DB_URL is unset, and update the skip reason/docstring accordingly.
Suggested change (DSN-only)
def _get_postgres_dsn() -> str | None:
"""Build PostgreSQL DSN from environment variables.
Primary source: ``OMNIBASE_INFRA_DB_URL``.
Fallback: individual ``POSTGRES_*`` env vars.
Returns:
PostgreSQL connection string, or None if not configured.
"""
db_url = os.getenv("OMNIBASE_INFRA_DB_URL")
- if db_url:
- return db_url
-
- host = os.getenv("POSTGRES_HOST")
- port = os.getenv("POSTGRES_PORT", "5432")
- user = os.getenv("POSTGRES_USER", "postgres")
- password = os.getenv("POSTGRES_PASSWORD")
-
- if not host or not password:
- return None
-
- return f"postgresql://{user}:{password}@{host}:{port}/omninode_bridge"
+ if not db_url:
+ return None
+ return db_url📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | |
| if db_url: | |
| return db_url | |
| host = os.getenv("POSTGRES_HOST") | |
| port = os.getenv("POSTGRES_PORT", "5432") | |
| database = os.getenv("POSTGRES_DATABASE", "omninode_bridge") | |
| user = os.getenv("POSTGRES_USER", "postgres") | |
| password = os.getenv("POSTGRES_PASSWORD") | |
| if not host or not password: | |
| return None | |
| return f"postgresql://{user}:{password}@{host}:{port}/{database}" | |
| return f"postgresql://{user}:{password}@{host}:{port}/omninode_bridge" | |
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | |
| if not db_url: | |
| return None | |
| return db_url |
🤖 Prompt for AI Agents
In `@tests/integration/services/snapshot/test_store_postgres_integration.py`
around lines 71 - 83, Replace the legacy POSTGRES_* fallback and hardcoded
"omninode_bridge" DSN by requiring OMNIBASE_INFRA_DB_URL only: if
os.getenv("OMNIBASE_INFRA_DB_URL") is set return it, otherwise return None (or
raise) immediately; remove the host/port/user/password logic and update any test
skip reason/docstring to state that OMNIBASE_INFRA_DB_URL is required. Ensure
references in the code use OMNIBASE_INFRA_DB_URL (not
POSTGRES_HOST/PORT/USER/PASSWORD) so the integration test fails fast when the
DSN is missing.
jonahgabriel
added a commit
that referenced
this pull request
Feb 10, 2026
Sanitize DSN in error messages to prevent credential leaks, remove silent fallback to legacy omninode_bridge database name, honor OMNIBASE_INFRA_DB_URL in availability checks, and align all test docstrings with the new per-service DB URL convention.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/integration/dlq/test_dlq_tracking_integration.py (1)
16-44:⚠️ Potential issue | 🟡 MinorUpdate skip reason to match DSN-first requirements.
The module doc now states OMNIBASE_INFRA_DB_URL is preferred, but the skip reason still says “POSTGRES_PASSWORD not set,” which can mislead CI output. Consider aligning the message with the DSN-first guidance.
Suggested fix
pytestmark = [ pytest.mark.skipif( not POSTGRES_AVAILABLE, - reason="PostgreSQL not available (POSTGRES_PASSWORD not set)", + reason=( + "PostgreSQL not available (set OMNIBASE_INFRA_DB_URL or " + "POSTGRES_HOST/POSTGRES_PASSWORD)" + ), ), ]Also applies to: 69-73
scripts/dlq_replay.py (1)
512-545:⚠️ Potential issue | 🟠 MajorFail fast when tracking is enabled but OMNIBASE_INFRA_DB_URL is missing.
Right now
--enable-trackingcan silently disable tracking, which contradicts the CLI help and surprises operators. Prefer a hard error when the flag is set but the DSN is absent/blank.🔧 Suggested fix
enable_tracking = getattr(args, "enable_tracking", False) - postgres_dsn = os.environ.get("OMNIBASE_INFRA_DB_URL") + postgres_dsn = os.environ.get("OMNIBASE_INFRA_DB_URL") + if enable_tracking: + postgres_dsn = postgres_dsn.strip() if postgres_dsn else "" + if not postgres_dsn: + raise ValueError( + "OMNIBASE_INFRA_DB_URL is required when --enable-tracking is set." + )
🤖 Fix all issues with AI agents
In `@scripts/backfill_capabilities.py`:
- Around line 75-78: The CFG_DB_001 message no longer matches the actual
validation (it checks DSN scheme, not database name); update the CFG_DB_001
description string in scripts/backfill_capabilities.py to reflect that it flags
an invalid DSN scheme (e.g., "Invalid DSN scheme") or alternatively extend the
validation logic in the function that emits CFG_DB_001 to also validate the
database-name format so the original message ("Invalid database name format")
remains accurate; locate references to CFG_DB_001 and the DSN/DB validation
logic (the code that parses/checks the DSN) and make the description and
validation consistent.
- Around line 215-244: The DSN validation in _get_validated_dsn currently only
checks scheme; update it to parse the DSN (e.g., via urllib.parse.urlparse) and
ensure the path component contains a non-empty database name (path must exist,
not just '/' and not empty after the leading slash) to avoid connecting to the
default/user DB; if missing, raise ConfigurationError with a clear message and a
new or existing error code (e.g., ErrorCode.CFG_MISSING_DB_NAME) so callers know
the DSN must include the target database.
In `@tests/helpers/util_postgres.py`:
- Around line 258-261: The warning message in tests/helpers/util_postgres.py
hardcodes "OMNIBASE_INFRA_DB_URL" which can mislead when a different env var is
used; update the logger.warning call to interpolate the db_url_var variable
instead of the hardcoded name (locate the logger.warning invocation and replace
the literal string with one that includes db_url_var), ensuring the message
still states that the URL is set but contains no database name.
In `@tests/integration/handlers/conftest.py`:
- Around line 192-207: The environment variable _OMNIBASE_INFRA_DB_URL should be
normalized (strip whitespace and treat empty/whitespace-only as None) before
computing POSTGRES_AVAILABLE; update the initialization of
_OMNIBASE_INFRA_DB_URL to strip() and set to None if falsy (e.g.,
_OMNIBASE_INFRA_DB_URL = os.getenv("OMNIBASE_INFRA_DB_URL"); then normalize with
_OMNIBASE_INFRA_DB_URL = _OMNIBASE_INFRA_DB_URL.strip() or None) and ensure
POSTGRES_AVAILABLE uses the normalized _OMNIBASE_INFRA_DB_URL and the existing
POSTGRES_HOST/POSTGRES_PASSWORD logic so whitespace-only DSNs are not considered
available.
🧹 Nitpick comments (3)
tests/integration/services/snapshot/test_store_postgres_integration.py (1)
22-34: Documentation still references fallback that PR objectives say should be removed.The docstring documents the POSTGRES_* fallback behavior (lines 26-30), but if the fallback is removed per the PR's fail-fast requirement, this documentation should be simplified to only mention
OMNIBASE_INFRA_DB_URL.tests/integration/handlers/test_registration_storage_handler_swapping.py (1)
82-106: Preserve DSN query params (sslmode, options) when OMNIBASE_INFRA_DB_URL is used.Parsing into host/port/user/password drops DSN query parameters, which can be required for TLS or special server options. Consider passing the DSN through directly (if the handler supports it) or propagating query params into the handler config.
tests/integration/runtime/test_projector_shell_database.py (1)
116-138: Encode credentials in fallback DSN and trim OMNIBASE_INFRA_DB_URL.If
POSTGRES_USER/POSTGRES_PASSWORDcontain@,:or%, the DSN becomes invalid. Also, a whitespace-only OMNIBASE_INFRA_DB_URL should be treated as unset.🔧 Suggested fix
import os +from urllib.parse import quote_plus @@ - db_url = os.getenv("OMNIBASE_INFRA_DB_URL") - if db_url: - return db_url + db_url = os.getenv("OMNIBASE_INFRA_DB_URL") + if db_url: + db_url = db_url.strip() + if db_url: + return db_url @@ - return f"postgresql://{user}:{password}@{host}:{port}/omnibase_infra" + encoded_user = quote_plus(user, safe="") + encoded_password = quote_plus(password, safe="") + return f"postgresql://{encoded_user}:{encoded_password}@{host}:{port}/omnibase_infra"
Comment on lines
+192
to
+207
| # Read configuration from environment variables (set via docker-compose or .env) | ||
| # Primary: OMNIBASE_INFRA_DB_URL (full DSN). Fallback: individual POSTGRES_* vars. | ||
| _OMNIBASE_INFRA_DB_URL = os.getenv("OMNIBASE_INFRA_DB_URL") | ||
| POSTGRES_HOST = os.getenv("POSTGRES_HOST") | ||
| POSTGRES_PORT = os.getenv("POSTGRES_PORT", "5432") | ||
| POSTGRES_DATABASE = os.getenv("POSTGRES_DATABASE", "omninode_bridge") | ||
| POSTGRES_USER = os.getenv("POSTGRES_USER", "postgres") | ||
| POSTGRES_PASSWORD = os.getenv("POSTGRES_PASSWORD") | ||
|
|
||
| # Defensive check: warn if POSTGRES_PASSWORD is missing or empty to avoid silent failures | ||
| # Handles None, empty string, and whitespace-only values | ||
| if not POSTGRES_PASSWORD or not POSTGRES_PASSWORD.strip(): | ||
| import warnings | ||
|
|
||
| warnings.warn( | ||
| "POSTGRES_PASSWORD environment variable not set or empty - database integration " | ||
| "tests will be skipped. Set POSTGRES_PASSWORD in your .env file or environment " | ||
| "to enable database tests.", | ||
| UserWarning, | ||
| stacklevel=1, | ||
| ) | ||
| # Normalize to None for consistent availability check | ||
| # Normalize empty/whitespace-only password to None for consistent availability check | ||
| if POSTGRES_PASSWORD and not POSTGRES_PASSWORD.strip(): | ||
| POSTGRES_PASSWORD = None | ||
|
|
||
| # Check if PostgreSQL is available based on host and password being set | ||
| POSTGRES_AVAILABLE = POSTGRES_HOST is not None and POSTGRES_PASSWORD is not None | ||
| # Check if PostgreSQL is available based on URL or host+password being set | ||
| POSTGRES_AVAILABLE = bool(_OMNIBASE_INFRA_DB_URL) or ( | ||
| POSTGRES_HOST is not None and POSTGRES_PASSWORD is not None | ||
| ) |
There was a problem hiding this comment.
Normalize OMNIBASE_INFRA_DB_URL whitespace before availability checks.
A whitespace-only DSN currently evaluates as available and will cause confusing connection failures.
🔧 Suggested fix
-_OMNIBASE_INFRA_DB_URL = os.getenv("OMNIBASE_INFRA_DB_URL")
+_OMNIBASE_INFRA_DB_URL = os.getenv("OMNIBASE_INFRA_DB_URL")
+if _OMNIBASE_INFRA_DB_URL is not None:
+ _OMNIBASE_INFRA_DB_URL = _OMNIBASE_INFRA_DB_URL.strip() or None🤖 Prompt for AI Agents
In `@tests/integration/handlers/conftest.py` around lines 192 - 207, The
environment variable _OMNIBASE_INFRA_DB_URL should be normalized (strip
whitespace and treat empty/whitespace-only as None) before computing
POSTGRES_AVAILABLE; update the initialization of _OMNIBASE_INFRA_DB_URL to
strip() and set to None if falsy (e.g., _OMNIBASE_INFRA_DB_URL =
os.getenv("OMNIBASE_INFRA_DB_URL"); then normalize with _OMNIBASE_INFRA_DB_URL =
_OMNIBASE_INFRA_DB_URL.strip() or None) and ensure POSTGRES_AVAILABLE uses the
normalized _OMNIBASE_INFRA_DB_URL and the existing
POSTGRES_HOST/POSTGRES_PASSWORD logic so whitespace-only DSNs are not considered
available.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/omnibase_infra/cli/commands.py`:
- Around line 205-227: In _get_db_dsn generate or capture correlation_id once
(use correlation_id = uuid4()) before any ClickException and include that same
correlation_id in the invalid-scheme error message so both error paths contain
the correlation id; specifically, move/create correlation_id at top of
_get_db_dsn (or before the scheme check) and interpolate it into the second
click.ClickException string for easier tracing.
In `@tests/integration/handlers/test_registration_storage_handler_swapping.py`:
- Around line 74-102: Update the misleading comment inside
_resolve_postgres_config: remove or rewrite the sentence that claims
PostgresConfig.from_env() "returns empty database when using individual
POSTGRES_* env vars" and instead state that PostgresConfig.from_env() already
defaults the database to "omnibase_infra"; keep the rest of the fallback logic
intact and reference PostgresConfig.from_env (tests/helpers/util_postgres.py) so
readers know the authoritative source of the default.
In `@tests/integration/ledger/conftest.py`:
- Around line 45-56: The DSN validation in the db URL retrieval currently checks
only the scheme but not that a database name is present; update the block that
parses OMNIBASE_INFRA_DB_URL (the code using urlparse and the local variables
db_url and parsed) to also verify parsed.path contains a database name (i.e.
parsed.path exists and parsed.path != '/'); if not, raise a ValueError
explaining that OMNIBASE_INFRA_DB_URL must include an explicit database name to
avoid connecting tests to a default DB. Ensure the error message clearly
includes the supplied scheme/path for debugging.
In `@tests/integration/services/snapshot/test_store_postgres_integration.py`:
- Around line 61-90: The current _get_postgres_dsn function accepts
OMNIBASE_INFRA_DB_URL without verifying it contains a database name or a valid
postgres scheme; parse db_url (use urllib.parse.urlparse) and validate that
parsed.scheme indicates Postgres (e.g., startswith "postgres") and that
parsed.path is non-empty and not just "/" (i.e., contains the DB name); if
either check fails, treat it as not configured (return None) so the function
falls back to the POSTGRES_* env-vars or disables tests.
🧹 Nitpick comments (2)
src/omnibase_infra/services/session/config_store.py (1)
98-124: Credentials are not URL-encoded in DSN construction.The
dsn,dsn_async, anddsn_safeproperties directly interpolatepostgres_userandpasswordinto the connection string without URL-encoding. If credentials contain reserved URI characters (e.g.,@,:,/,%,#), the resulting DSN will be malformed or misinterpreted.The PR summary mentions "add percent-decoding of DSN credentials and URL-encoding in fallback paths" was applied elsewhere—consider applying URL-encoding here for consistency and robustness.
♻️ Proposed fix to URL-encode credentials
+from urllib.parse import quote_plus + from pydantic import Field, SecretStr, model_validator from pydantic_settings import BaseSettings, SettingsConfigDictThen update the
dsnproperty (and similarly fordsn_async):`@property` def dsn(self) -> str: """Build PostgreSQL DSN from components. Returns: PostgreSQL connection string. """ password = self.postgres_password.get_secret_value() + encoded_user = quote_plus(self.postgres_user) + encoded_password = quote_plus(password) return ( - f"postgresql://{self.postgres_user}:{password}" + f"postgresql://{encoded_user}:{encoded_password}" f"@{self.postgres_host}:{self.postgres_port}" f"/{self.postgres_database}" )tests/integration/handlers/conftest.py (1)
233-243: Consider movingurlparseimport to module level for consistency.
quote_plusis imported at module level (line 107), buturlparseis imported inside the function. For consistency and slight performance improvement (avoiding repeated import lookups), consider co-locating the import.♻️ Suggested refactor
At the top of the file (around line 107):
-from urllib.parse import quote_plus +from urllib.parse import quote_plus, urlparseThen in the function:
if _OMNIBASE_INFRA_DB_URL: # Basic validation: ensure the user-provided DSN is well-formed - from urllib.parse import urlparse - parsed = urlparse(_OMNIBASE_INFRA_DB_URL)
Comment on lines
+205
to
+227
| def _get_db_dsn() -> str: | ||
| """Build PostgreSQL DSN from environment variables.""" | ||
| host = os.environ.get("POSTGRES_HOST", "192.168.86.200") | ||
| port = os.environ.get("POSTGRES_PORT", "5436") | ||
| user = os.environ.get("POSTGRES_USER", "postgres") | ||
| password = os.environ.get("POSTGRES_PASSWORD", "") | ||
| database = os.environ.get("POSTGRES_DATABASE", "omninode_bridge") | ||
| if not password: | ||
| click.echo( | ||
| "Warning: POSTGRES_PASSWORD not set. Set it via environment or .env file.", | ||
| err=True, | ||
| """Get PostgreSQL DSN from OMNIBASE_INFRA_DB_URL. | ||
|
|
||
| Raises: | ||
| click.ClickException: If OMNIBASE_INFRA_DB_URL is not set (fail-fast). | ||
| """ | ||
| db_url = os.environ.get("OMNIBASE_INFRA_DB_URL") | ||
| if not db_url: | ||
| correlation_id = uuid4() | ||
| raise click.ClickException( | ||
| f"OMNIBASE_INFRA_DB_URL is required but not set " | ||
| f"(correlation_id={correlation_id}). " | ||
| "Set it to a PostgreSQL DSN, e.g. " | ||
| "postgresql://user:pass@host:5432/omnibase_infra" | ||
| ) | ||
| return f"postgresql://{quote_plus(user)}:{quote_plus(password)}@{host}:{port}/{database}" | ||
|
|
||
| # Validate DSN scheme to catch obvious misconfigurations early | ||
| if not db_url.startswith(("postgresql://", "postgres://")): | ||
| raise click.ClickException( | ||
| f"OMNIBASE_INFRA_DB_URL has invalid scheme. " | ||
| f"Expected 'postgresql://' or 'postgres://', " | ||
| f"got: {db_url.split('://', 1)[0] if '://' in db_url else '(none)'}://" | ||
| ) |
There was a problem hiding this comment.
Include correlation_id in the invalid-scheme error message.
The scheme‑validation error path currently omits correlation_id, so failures aren’t fully traceable.
Proposed fix
def _get_db_dsn() -> str:
+ correlation_id = uuid4()
"""Get PostgreSQL DSN from OMNIBASE_INFRA_DB_URL.
@@
db_url = os.environ.get("OMNIBASE_INFRA_DB_URL")
if not db_url:
- correlation_id = uuid4()
raise click.ClickException(
f"OMNIBASE_INFRA_DB_URL is required but not set "
f"(correlation_id={correlation_id}). "
"Set it to a PostgreSQL DSN, e.g. "
"postgresql://user:pass@host:5432/omnibase_infra"
)
@@
if not db_url.startswith(("postgresql://", "postgres://")):
raise click.ClickException(
f"OMNIBASE_INFRA_DB_URL has invalid scheme. "
f"Expected 'postgresql://' or 'postgres://', "
- f"got: {db_url.split('://', 1)[0] if '://' in db_url else '(none)'}://"
+ f"got: {db_url.split('://', 1)[0] if '://' in db_url else '(none)'}:// "
+ f"(correlation_id={correlation_id})"
)As per coding guidelines "Propagate correlation_id from incoming requests; auto-generate with uuid4() if missing; include in all error context".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _get_db_dsn() -> str: | |
| """Build PostgreSQL DSN from environment variables.""" | |
| host = os.environ.get("POSTGRES_HOST", "192.168.86.200") | |
| port = os.environ.get("POSTGRES_PORT", "5436") | |
| user = os.environ.get("POSTGRES_USER", "postgres") | |
| password = os.environ.get("POSTGRES_PASSWORD", "") | |
| database = os.environ.get("POSTGRES_DATABASE", "omninode_bridge") | |
| if not password: | |
| click.echo( | |
| "Warning: POSTGRES_PASSWORD not set. Set it via environment or .env file.", | |
| err=True, | |
| """Get PostgreSQL DSN from OMNIBASE_INFRA_DB_URL. | |
| Raises: | |
| click.ClickException: If OMNIBASE_INFRA_DB_URL is not set (fail-fast). | |
| """ | |
| db_url = os.environ.get("OMNIBASE_INFRA_DB_URL") | |
| if not db_url: | |
| correlation_id = uuid4() | |
| raise click.ClickException( | |
| f"OMNIBASE_INFRA_DB_URL is required but not set " | |
| f"(correlation_id={correlation_id}). " | |
| "Set it to a PostgreSQL DSN, e.g. " | |
| "postgresql://user:pass@host:5432/omnibase_infra" | |
| ) | |
| return f"postgresql://{quote_plus(user)}:{quote_plus(password)}@{host}:{port}/{database}" | |
| # Validate DSN scheme to catch obvious misconfigurations early | |
| if not db_url.startswith(("postgresql://", "postgres://")): | |
| raise click.ClickException( | |
| f"OMNIBASE_INFRA_DB_URL has invalid scheme. " | |
| f"Expected 'postgresql://' or 'postgres://', " | |
| f"got: {db_url.split('://', 1)[0] if '://' in db_url else '(none)'}://" | |
| ) | |
| def _get_db_dsn() -> str: | |
| """Get PostgreSQL DSN from OMNIBASE_INFRA_DB_URL. | |
| Raises: | |
| click.ClickException: If OMNIBASE_INFRA_DB_URL is not set (fail-fast). | |
| """ | |
| correlation_id = uuid4() | |
| db_url = os.environ.get("OMNIBASE_INFRA_DB_URL") | |
| if not db_url: | |
| raise click.ClickException( | |
| f"OMNIBASE_INFRA_DB_URL is required but not set " | |
| f"(correlation_id={correlation_id}). " | |
| "Set it to a PostgreSQL DSN, e.g. " | |
| "postgresql://user:pass@host:5432/omnibase_infra" | |
| ) | |
| # Validate DSN scheme to catch obvious misconfigurations early | |
| if not db_url.startswith(("postgresql://", "postgres://")): | |
| raise click.ClickException( | |
| f"OMNIBASE_INFRA_DB_URL has invalid scheme. " | |
| f"Expected 'postgresql://' or 'postgres://', " | |
| f"got: {db_url.split('://', 1)[0] if '://' in db_url else '(none)'}:// " | |
| f"(correlation_id={correlation_id})" | |
| ) |
🤖 Prompt for AI Agents
In `@src/omnibase_infra/cli/commands.py` around lines 205 - 227, In _get_db_dsn
generate or capture correlation_id once (use correlation_id = uuid4()) before
any ClickException and include that same correlation_id in the invalid-scheme
error message so both error paths contain the correlation id; specifically,
move/create correlation_id at top of _get_db_dsn (or before the scheme check)
and interpolate it into the second click.ClickException string for easier
tracing.
Comment on lines
+74
to
+102
| POSTGRES_AVAILABLE = os.getenv("OMNIBASE_INFRA_DB_URL") is not None or ( | ||
| os.getenv("POSTGRES_HOST") is not None | ||
| and os.getenv("POSTGRES_PASSWORD") is not None | ||
| ) | ||
|
|
||
|
|
||
| def _resolve_postgres_config() -> dict[str, object]: | ||
| """Resolve PostgreSQL connection config from env vars. | ||
|
|
||
| Delegates to the shared ``PostgresConfig.from_env()`` utility to avoid | ||
| duplicating DSN parsing logic. See ``tests/helpers/util_postgres.py``. | ||
|
|
||
| Returns: | ||
| Dict with host, port, database, user, password keys. | ||
| """ | ||
| from tests.helpers.util_postgres import PostgresConfig | ||
|
|
||
| config = PostgresConfig.from_env() | ||
| return { | ||
| "host": config.host or "localhost", | ||
| "port": config.port, | ||
| # Fallback to "omnibase_infra" is intentional: PostgresConfig.from_env() | ||
| # returns empty database when using individual POSTGRES_* env vars (by | ||
| # design - the database must come from the URL). This test module always | ||
| # targets the omnibase_infra database. | ||
| "database": config.database or "omnibase_infra", | ||
| "user": config.user, | ||
| "password": config.password or "", | ||
| } |
There was a problem hiding this comment.
Fallback-database note is now misleading.
PostgresConfig.from_env already defaults the fallback database to “omnibase_infra”, so the comment about returning an empty database is outdated.
📝 Suggested fix
- # Fallback to "omnibase_infra" is intentional: PostgresConfig.from_env()
- # returns empty database when using individual POSTGRES_* env vars (by
- # design - the database must come from the URL). This test module always
- # targets the omnibase_infra database.
+ # Fallback to "omnibase_infra" is intentional: PostgresConfig.from_env()
+ # defaults the database to "omnibase_infra" when using individual
+ # POSTGRES_* env vars. This test module always targets that database.🤖 Prompt for AI Agents
In `@tests/integration/handlers/test_registration_storage_handler_swapping.py`
around lines 74 - 102, Update the misleading comment inside
_resolve_postgres_config: remove or rewrite the sentence that claims
PostgresConfig.from_env() "returns empty database when using individual
POSTGRES_* env vars" and instead state that PostgresConfig.from_env() already
defaults the database to "omnibase_infra"; keep the rest of the fallback logic
intact and reference PostgresConfig.from_env (tests/helpers/util_postgres.py) so
readers know the authoritative source of the default.
Comment on lines
+45
to
+56
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | ||
| if db_url: | ||
| # Basic validation: ensure the user-provided DSN is well-formed | ||
| from urllib.parse import urlparse | ||
|
|
||
| parsed = urlparse(db_url) | ||
| if parsed.scheme not in ("postgresql", "postgres"): | ||
| raise ValueError( | ||
| f"OMNIBASE_INFRA_DB_URL has invalid scheme '{parsed.scheme}'. " | ||
| "Expected 'postgresql://' or 'postgres://'." | ||
| ) | ||
| return db_url |
There was a problem hiding this comment.
Validate that OMNIBASE_INFRA_DB_URL includes a database name.
If the DSN has no path, asyncpg will connect to a default DB (often the user name), which can run destructive integration tests against the wrong database.
Proposed fix
parsed = urlparse(db_url)
if parsed.scheme not in ("postgresql", "postgres"):
raise ValueError(
f"OMNIBASE_INFRA_DB_URL has invalid scheme '{parsed.scheme}'. "
"Expected 'postgresql://' or 'postgres://'."
)
+ database = (parsed.path or "").lstrip("/")
+ if not database:
+ raise ValueError(
+ "OMNIBASE_INFRA_DB_URL must include a database name "
+ "(e.g., postgresql://user:pass@host:5432/omnibase_infra)."
+ )
return db_url📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | |
| if db_url: | |
| # Basic validation: ensure the user-provided DSN is well-formed | |
| from urllib.parse import urlparse | |
| parsed = urlparse(db_url) | |
| if parsed.scheme not in ("postgresql", "postgres"): | |
| raise ValueError( | |
| f"OMNIBASE_INFRA_DB_URL has invalid scheme '{parsed.scheme}'. " | |
| "Expected 'postgresql://' or 'postgres://'." | |
| ) | |
| return db_url | |
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | |
| if db_url: | |
| # Basic validation: ensure the user-provided DSN is well-formed | |
| from urllib.parse import urlparse | |
| parsed = urlparse(db_url) | |
| if parsed.scheme not in ("postgresql", "postgres"): | |
| raise ValueError( | |
| f"OMNIBASE_INFRA_DB_URL has invalid scheme '{parsed.scheme}'. " | |
| "Expected 'postgresql://' or 'postgres://'." | |
| ) | |
| database = (parsed.path or "").lstrip("/") | |
| if not database: | |
| raise ValueError( | |
| "OMNIBASE_INFRA_DB_URL must include a database name " | |
| "(e.g., postgresql://user:pass@host:5432/omnibase_infra)." | |
| ) | |
| return db_url |
🤖 Prompt for AI Agents
In `@tests/integration/ledger/conftest.py` around lines 45 - 56, The DSN
validation in the db URL retrieval currently checks only the scheme but not that
a database name is present; update the block that parses OMNIBASE_INFRA_DB_URL
(the code using urlparse and the local variables db_url and parsed) to also
verify parsed.path contains a database name (i.e. parsed.path exists and
parsed.path != '/'); if not, raise a ValueError explaining that
OMNIBASE_INFRA_DB_URL must include an explicit database name to avoid connecting
tests to a default DB. Ensure the error message clearly includes the supplied
scheme/path for debugging.
Comment on lines
+61
to
+90
| def _get_postgres_dsn() -> str | None: | ||
| """Build PostgreSQL DSN from environment variables. | ||
|
|
||
| Primary source: ``OMNIBASE_INFRA_DB_URL``. | ||
| Fallback: individual ``POSTGRES_*`` env vars. | ||
|
|
||
| Returns: | ||
| PostgreSQL connection string, or None if required vars not set. | ||
| PostgreSQL connection string, or None if not configured. | ||
| """ | ||
| db_url = os.getenv("OMNIBASE_INFRA_DB_URL") | ||
| if db_url: | ||
| return db_url | ||
|
|
||
| from urllib.parse import quote_plus | ||
|
|
||
| host = os.getenv("POSTGRES_HOST") | ||
| port = os.getenv("POSTGRES_PORT", "5432") | ||
| database = os.getenv("POSTGRES_DATABASE", "omninode_bridge") | ||
| user = os.getenv("POSTGRES_USER", "postgres") | ||
| password = os.getenv("POSTGRES_PASSWORD") | ||
|
|
||
| if not host or not password: | ||
| return None | ||
|
|
||
| return f"postgresql://{user}:{password}@{host}:{port}/{database}" | ||
| # URL-encode credentials to handle special characters (@, :, /, %, etc.) | ||
| encoded_user = quote_plus(user, safe="") | ||
| encoded_password = quote_plus(password, safe="") | ||
|
|
||
| return ( | ||
| f"postgresql://{encoded_user}:{encoded_password}@{host}:{port}/omnibase_infra" | ||
| ) |
There was a problem hiding this comment.
Guard against DSNs missing a database name.
If OMNIBASE_INFRA_DB_URL has no path, tests may run against a default DB. Add a database-name check (and scheme validation) before accepting the DSN.
Proposed fix
db_url = os.getenv("OMNIBASE_INFRA_DB_URL")
if db_url:
+ from urllib.parse import urlparse
+
+ parsed = urlparse(db_url)
+ if parsed.scheme not in ("postgresql", "postgres"):
+ raise ValueError(
+ f"OMNIBASE_INFRA_DB_URL has invalid scheme '{parsed.scheme}'. "
+ "Expected 'postgresql://' or 'postgres://'."
+ )
+ database = (parsed.path or "").lstrip("/")
+ if not database:
+ raise ValueError(
+ "OMNIBASE_INFRA_DB_URL must include a database name "
+ "(e.g., postgresql://user:pass@host:5432/omnibase_infra)."
+ )
return db_url🤖 Prompt for AI Agents
In `@tests/integration/services/snapshot/test_store_postgres_integration.py`
around lines 61 - 90, The current _get_postgres_dsn function accepts
OMNIBASE_INFRA_DB_URL without verifying it contains a database name or a valid
postgres scheme; parse db_url (use urllib.parse.urlparse) and validate that
parsed.scheme indicates Postgres (e.g., startswith "postgres") and that
parsed.path is non-empty and not just "/" (i.e., contains the DB name); if
either check fails, treat it as not configured (return None) so the function
falls back to the POSTGRES_* env-vars or disables tests.
jonahgabriel
added a commit
that referenced
this pull request
Feb 10, 2026
- Add correlation_id to invalid-scheme error in CLI commands - Add DSN database-name validation in ledger, snapshot, and projector integration test conftest files to catch misconfigured URLs early - Normalize whitespace on OMNIBASE_INFRA_DB_URL in handler conftest and handler-swapping test to prevent false-positive availability - Improve util_postgres warning to show expected DSN format when database name is missing - Move urlparse imports to module level in handlers/conftest and backfill_capabilities script - Reorder .env.example keys alphabetically per dotenv-linter - Update docs to describe POSTGRES_HOST/PASSWORD as fallback, not required, now that OMNIBASE_INFRA_DB_URL is primary - Add security comment in pool config confirming credential sanitization - Add TODO for DSN query-param preservation and parsing deduplication
jonahgabriel
added a commit
that referenced
this pull request
Feb 10, 2026
Deduplicate DSN parsing in snapshot and ledger test conftest files by delegating to shared PostgresConfig utility (-70 lines). Update skip reasons to mention OMNIBASE_INFRA_DB_URL as the primary config method, fix misleading comments about PostgresConfig defaults, and clarify fallback error messages.
jonahgabriel
added a commit
that referenced
this pull request
Feb 10, 2026
- Validate DSN contains a database name before setting POSTGRES_AVAILABLE in handler conftest and handler swapping tests (prevents false positives) - Add safety warning in backfill script when database name != omnibase_infra - Improve util_postgres warning to show parsed path and explicit consequence - Fix DLQ conftest docstring port (5436→5432 for localhost example) - Alphabetize .env.example pool config keys per dotenv-linter - Add agent-actions-consumer DB URL cross-reference in .env.example - Update misleading fallback comment in handler swapping test
Migrate from individual POSTGRES_* env vars to per-service *_DB_URL DSN pattern (DB-SPLIT-02). This eliminates the shared POSTGRES_DATABASE=omninode_bridge reference and introduces fail-fast behavior when OMNIBASE_INFRA_DB_URL is not set. - Add all 6 *_DB_URL vars to .env.example files - ModelPostgresPoolConfig.from_env() now parses OMNIBASE_INFRA_DB_URL - Add from_dsn() classmethod for programmatic DSN parsing - CLI, plugin, scripts all read OMNIBASE_INFRA_DB_URL directly - Docker-compose uses hardcoded DB names for container init - Update all 31 files across src, tests, docker, scripts, docs
- Sanitize password from DSN before including in error messages - Add test_from_env_missing_url_raises for fail-fast behavior - Add test_from_dsn_missing_database_raises for DSN validation - Add test_from_dsn_missing_database_sanitizes_password for security
The migration from individual POSTGRES_* env vars to a single DSN broke Docker integration tests that launch containers via raw `docker run` without the new required env var.
Sanitize DSN in error messages to prevent credential leaks, remove silent fallback to legacy omninode_bridge database name, honor OMNIBASE_INFRA_DB_URL in availability checks, and align all test docstrings with the new per-service DB URL convention.
…odes Major fixes: - Fix 4 fallback database names: omninode_bridge -> omnibase_infra in test conftest files (handlers, e2e, runtime/db, snapshot) - Update .secretresolver_allowlist with correct plugin.py line numbers and env var names after OMNIBASE_INFRA_DB_URL migration - Rename CFG_MISSING_PASSWORD -> CFG_MISSING_DB_URL error code in backfill_capabilities.py to match actual condition Minor fixes: - Update 5 docstring examples from omninode_bridge to omnibase_infra - Clarify POSTGRES_HOST/PORT/USER as legacy fallbacks in docker/README.md - Remove unreachable try/except around urlparse() in from_dsn() - Remove stale CFG_HOST/PORT/USER error codes from backfill docstring - Fix stale hint message in util_postgres.py build_dsn() - Deduplicate DSN parsing in test_registration_storage_handler_swapping.py Review iteration: 1/10
- Delete ~110 lines of dead code: _validate_hostname(), _validate_port(), _validate_identifier() and their unused error codes (CFG_INVALID_HOST, CFG_INVALID_PORT, CFG_INVALID_DATABASE) from backfill_capabilities.py - Remove unused ipaddress and re imports - Fix stale skip reason in test_store_postgres_integration.py to mention OMNIBASE_INFRA_DB_URL - Fix stale skip message in test_projector_shell_database.py to mention OMNIBASE_INFRA_DB_URL Review iteration: 2/10
- Update default database from 'omninode_bridge' to 'omnibase_infra' in handler_registration_storage_postgres and config_store - Add DSN scheme validation in backfill_capabilities._get_validated_dsn() - Add fail-fast :? syntax for POSTGRES_PASSWORD in docker-compose.infra.yml nested variable fallback for OMNIBASE_INFRA_DB_URL - Move OMNIBASE_INFRA_DB_URL to Required Variables section in docker README - Document port 5436 fallback rationale in ledger conftest - Update test assertion for new default database name Review iteration: 1/10
- Update backfill error guidance to reference OMNIBASE_INFRA_DB_URL instead of legacy POSTGRES_HOST/PORT/PASSWORD vars - Fix e2e conftest docstring: omninode_bridge -> omnibase_infra - Fix test database_ref: omninode_bridge -> omnibase_infra - Remove redundant omnibase_infra from POSTGRES_MULTIPLE_DATABASES in e2e compose (already created as POSTGRES_DB) Review iteration: 2/10
…INFRA_DB_URL - Replace POSTGRES_HOST:5436 (database: omninode_bridge) with OMNIBASE_INFRA_DB_URL (database: omnibase_infra) in test docstring Review iteration: 3/10
… stale references - Fix db_config/dlq_tracking_config fixtures incorrectly skipping when only OMNIBASE_INFRA_DB_URL is set (POSTGRES_PASSWORD guard → POSTGRES_AVAILABLE) - Remove misleading POSTGRES_PASSWORD warning when URL provides credentials - Add DSN scheme validation and percent-encoded password decoding in from_dsn() - Add int() validation for POSTGRES_POOL_MIN/MAX_SIZE env vars - Add .strip() on DSN in backfill_capabilities.py - Add explicit warning when --enable-tracking used without OMNIBASE_INFRA_DB_URL - Fix .env.example header: rand -base64 → rand -hex for URI-safe passwords - Remove stale omninode_bridge from docker-compose.infra.yml MULTIPLE_DATABASES - Update 10 test files with stale docstrings/skip messages to reference OMNIBASE_INFRA_DB_URL as the primary configuration
…y docs - Fix urlparse credential double-encoding bug in PostgresConfig.from_env() and _resolve_postgres_config() by adding unquote() (matching model_postgres_pool_config.py pattern) - Add database path validation to backfill_capabilities.py DSN checker - Remove dead module-level POSTGRES_HOST/POSTGRES_PASSWORD vars - Add clarifying comments to docker-compose files about legacy env vars and nested variable expansion edge cases - Document config_store.py intentional deviation from DSN migration Review iteration: 1/10
_build_postgres_dsn() now delegates to PostgresConfig.build_dsn() which self-validates; update docstring to reflect actual behavior. Review iteration: 2/10
- Add DSN scheme validation when OMNIBASE_INFRA_DB_URL is provided directly in test conftest files (handlers, e2e, ledger) to catch malformed URLs early - Allow host-level OMNIBASE_INFRA_DB_URL override in docker-compose.e2e.yml runtime service (was hardcoded, preventing env-based override) - Replace inline _resolve_postgres_config() in handler swapping tests with shared PostgresConfig.from_env() from tests/helpers/util_postgres.py - Rename misleading CFG_AUTH_001 error code to CFG_URL_001 in backfill script (missing DB URL is a config error, not an auth error) - Clarify ModelPostgresPoolConfig.database field requires explicit value - Fix hardcoded var name in util_postgres.py warning (use db_url_var param) - Add comment documenting intentional hardcoded db name per OMN-2065 migration Review iteration: 1/10
…ation - Add quote_plus() URL-encoding to fallback DSN construction in 4 test conftest files (ledger, snapshot, projector_shell, postgres_repository) to match the encoding already used in handlers/conftest.py and e2e/conftest.py - prevents malformed DSNs with special characters - Add DSN scheme validation to CLI helpers (_helpers.py get_postgres_dsn and commands.py _get_db_dsn) to catch obvious misconfigurations early - Document intentional "omnibase_infra" database fallback in handler swapping test's _resolve_postgres_config() - Fix _urlparse alias in e2e/conftest.py for consistency Review iteration: 2/10
- Restore POSTGRES_HOST/POSTGRES_PASSWORD fallback in cleanup_postgres_test_projections fixture (tests/conftest.py) so test cleanup works with either OMNIBASE_INFRA_DB_URL or individual vars - Split CFG_INVALID_DSN_SCHEME error code into CFG_SCHEME_001 (invalid scheme) and CFG_MISSING_DB_NAME/CFG_DB_001 (missing database name) in backfill script for clearer operator diagnostics Review iteration: 3/10
…igured The PostgresConfig.from_env() fallback path (individual POSTGRES_* vars) was setting database="" which made is_configured return False, silently skipping all database integration tests for environments using the fallback instead of OMNIBASE_INFRA_DB_URL. Default to "omnibase_infra". Review iteration: 1/10
Add 'from e' to ValueError re-raises in from_env() pool size parsing to preserve the original traceback for debugging. Review iteration: 1/10
…_infra Update 14 stale docstring/test references to the old database name across observability consumers, __init__.py files, TTL cleanup config, health check details model, and consumer test fixtures. Review iteration: 1/10
…'t test it Tests in TestIntegration (test_kernel.py) and TestRuntimeHostProcessContractConfig (test_runtime_host_process.py) exercise bootstrap and contract config loading respectively. They were failing in CI because _materialize_dependencies eagerly reads OMNIBASE_INFRA_DB_URL via ModelPostgresPoolConfig.from_env(). Patching as a no-op since materialization has its own dedicated test suite.
- Add correlation_id to invalid-scheme error in CLI commands - Add DSN database-name validation in ledger, snapshot, and projector integration test conftest files to catch misconfigured URLs early - Normalize whitespace on OMNIBASE_INFRA_DB_URL in handler conftest and handler-swapping test to prevent false-positive availability - Improve util_postgres warning to show expected DSN format when database name is missing - Move urlparse imports to module level in handlers/conftest and backfill_capabilities script - Reorder .env.example keys alphabetically per dotenv-linter - Update docs to describe POSTGRES_HOST/PASSWORD as fallback, not required, now that OMNIBASE_INFRA_DB_URL is primary - Add security comment in pool config confirming credential sanitization - Add TODO for DSN query-param preservation and parsing deduplication
Deduplicate DSN parsing in snapshot and ledger test conftest files by delegating to shared PostgresConfig utility (-70 lines). Update skip reasons to mention OMNIBASE_INFRA_DB_URL as the primary config method, fix misleading comments about PostgresConfig defaults, and clarify fallback error messages.
- Validate DSN contains a database name before setting POSTGRES_AVAILABLE in handler conftest and handler swapping tests (prevents false positives) - Add safety warning in backfill script when database name != omnibase_infra - Improve util_postgres warning to show parsed path and explicit consequence - Fix DLQ conftest docstring port (5436→5432 for localhost example) - Alphabetize .env.example pool config keys per dotenv-linter - Add agent-actions-consumer DB URL cross-reference in .env.example - Update misleading fallback comment in handler swapping test
…laceholder - Add URI-special characters to x-runtime-env warning (consistent with agent-actions) - Soften Compose version reference to "recent Compose v2" (unverified threshold) - Make .env.example placeholder instruction more explicit about replacement Review iteration: 2/10
…r DSN warning - Fixed: scripts/backfill_capabilities.py:245 - Use urlparse(dsn).scheme instead of str.split for consistent scheme extraction - Fixed: src/omnibase_infra/cli/commands.py:229 - Same urlparse consistency fix - Fixed: src/omnibase_infra/cli/infra_test/_helpers.py:63 - Same urlparse consistency fix - Fixed: docker/docker-compose.infra.yml:97 - Add production recommendation to set OMNIBASE_INFRA_DB_URL explicitly Review iteration: 1/10
…ntegration tests Integration tests for handler discovery, bootstrap source, and source mode resolution don't need a real PostgreSQL pool. Patch _materialize_dependencies to prevent ValueError when OMNIBASE_INFRA_DB_URL is unset in CI, matching the existing unit test pattern.
… utility - Refactor handlers/conftest.py to use PostgresConfig.from_env() (-80 lines) - Refactor e2e/conftest.py to use PostgresConfig.from_env() (-70 lines) - Refactor test_registration_storage_handler_swapping.py POSTGRES_AVAILABLE - Add override support for agent-actions DSN in docker-compose.infra.yml - Update backfill_capabilities.py TODO to note consolidation Review iteration: 1/10
…postgres_config Avoid redundant PostgresConfig.from_env() call inside the function body; reuse the module-level instance created at import time. Review iteration: 2/10
…rences, relax test guards - Strengthened Docker Compose fallback DSN warning: requires hex-safe passwords since Compose cannot URL-encode variables - Removed orphaned POSTGRES_HOST/POSTGRES_PORT from x-runtime-env (runtime code exclusively reads OMNIBASE_INFRA_DB_URL) - Clarified from_dsn() empty-string password design choice - Fixed conftest cleanup fallback port from 5436 to standard 5432 - Updated protocol_domain_plugin.py docstring: OMNIBASE_INFRA_DB_URL - Removed stale .secretresolver_allowlist entries for service_kernel.py POSTGRES_* vars (migrated to OMNIBASE_INFRA_DB_URL in OMN-2065) - Relaxed test_postgres_repository_runtime fallback check: only POSTGRES_HOST and POSTGRES_PASSWORD are truly required (port/user have defaults) Review iteration: 1/10
…consistency fixes - Warn when DSN query params (sslmode, etc.) are silently discarded in from_dsn() - Add scheme/database validation to test get_dsn() for parity with other consumers - Reuse single correlation_id in CLI _get_db_dsn() instead of generating three - Add DSN scheme/database validation in plugin.py before asyncpg.create_pool() - Use PostgresConfig.from_env() in conftest cleanup instead of inline DSN construction - Fix README: clarify OMNIBASE_INFRA_DB_URL is required for the runtime Review iteration: 1/10
…rify database field - PostgresConfig.from_env() now validates DSN scheme is postgresql/postgres (returns unconfigured config on invalid scheme, consistent with other consumers) - Clarify database field description: explicitly note it has no default Review iteration: 2/10
… error context - Move urlparse import to module level in plugin.py (consistency) - Move quote_plus import to module level in test files (consistency) - Extract shared pool_error_context in plugin.py DSN validation (DRY) Review iteration: 3/10
…wlist Plugin.py line shifts from DSN validation changes moved os.getenv() calls to different lines. Updated 5 allowlist entries to match. Review iteration: 4/10
…cy, and test dedup - Fixed: model_postgres_pool_config.py + 7 files - reject DSN sub-paths (e.g., mydb/schema) in database name extraction - Fixed: docker-compose.infra.yml:478 - hardcode postgres:5432 in agent-actions consumer fallback DSN, matching runtime anchor - Fixed: docker-compose.infra.yml:51 - document Compose v2.20+ requirement for nested variable expansion - Fixed: model_postgres_pool_config.py:148 - clarify DSN query params warning (preserved when DSN passed directly to asyncpg) - Fixed: model_postgres_pool_config.py:37 - use explicit Field(...) for required database field - Fixed: tests/helpers/util_postgres.py:289 - document empty-string username edge case - Fixed: docker-compose.e2e.yml:275 - document POSTGRES_PASSWORD fallback difference vs infra compose - Fixed: Extract _skip_materialize_dependencies fixture to root conftest (removed 11 duplicates across 5 test files) Review iteration: 1/10
…wlist - Fixed: .secretresolver_allowlist:21 - plugin.py:376→383 for ONEX_PROJECTOR_CONTRACTS_DIR - Fixed: .secretresolver_allowlist:22 - plugin.py:532→538 for CONSUL_HOST - Fixed: .secretresolver_allowlist:23 - plugin.py:543→549 for CONSUL_PORT Review iteration: 2/10
…en compose guards - Moved _skip_materialize_dependencies fixture from root conftest to tests/unit/conftest.py so integration tests are not silently mocked - Added :? guard to POSTGRES_PASSWORD in docker-compose.infra.yml for consistent fail-fast behavior with the DSN fallback - Removed unnecessary correlation_id from CLI DSN validation errors - Cleaned up redundant os.getenv default in plugin.py initialize() - Documented password-less DSN edge case in util_postgres.py Review iteration: 1/10
- Added ModelPostgresPoolConfig.validate_dsn() static method as the single source of truth for DSN scheme, database name, and sub-path validation - Refactored from_dsn() to delegate to validate_dsn() internally - Updated _helpers.py, commands.py, and plugin.py to use shared validation instead of duplicating the 3-check pattern - Added try/except for non-numeric port in util_postgres.py to prevent test collection crashes from malformed DSN values - Updated test regex to match new shared error message format Review iteration: 2/10
…wlist - Fixed: .secretresolver_allowlist:19 - plugin.py:225→228 (OMNIBASE_INFRA_DB_URL) - Fixed: .secretresolver_allowlist:20 - plugin.py:261→264 (OMNIBASE_INFRA_DB_URL) - Fixed: .secretresolver_allowlist:21 - plugin.py:383→374 (ONEX_PROJECTOR_CONTRACTS_DIR) - Fixed: .secretresolver_allowlist:22 - plugin.py:538→529 (CONSUL_HOST) - Fixed: .secretresolver_allowlist:23 - plugin.py:549→540 (CONSUL_PORT) Review iteration: 1/10
…ping - Fixed: model_postgres_pool_config.py:128 - validate_dsn() now strips whitespace - Fixed: backfill_capabilities.py:218 - Delegated to shared ModelPostgresPoolConfig.validate_dsn() - Fixed: backfill_capabilities.py:243 - Eliminated startswith() vs urlparse() inconsistency Review iteration: 2/10
- Fixed: config_store.py:116 - URL-encode database name and handle IPv6 hosts in DSN - Fixed: model_postgres_pool_config.py:197 - Document decoded credential storage - Fixed: dlq_replay.py:336 - Add postgres_dsn scheme validation - Fixed: docker-compose.e2e.yml:277 - Add URI-special char warning comment - Fixed: test_registration_storage_handler_swapping.py:104 - Document empty password safety - Fixed: docker-compose.infra.yml:100 - Document POSTGRES_USER/PASSWORD consumers - Fixed: tests/conftest.py:68 - Remove unused Generator and patch imports Review iteration: 2/10
… codes - Fixed: model_postgres_pool_config.py:173 - from_dsn() now uses stripped DSN from validate_dsn() - Fixed: backfill_capabilities.py:253 - Map database name errors to CFG_MISSING_DB_NAME code Review iteration: 3/10
- Fixed: backfill_capabilities.py:253 - Match on 'scheme' keyword (stable) instead of 'database' (fragile) for error code discrimination Review iteration: 4/10
… Compose docs - Refactored test_postgres_repository_runtime_integration.py to delegate DSN resolution to shared PostgresConfig.from_env() instead of duplicating scheme/database/sub-path validation inline - Refactored test_projector_shell_database.py similarly to use PostgresConfig - Made Docker Compose v2.20+ requirement more prominent in both compose files - Documented password-character restrictions in Docker fallback DSN (docker/README.md) - Clarified OMNIBASE_INFRA_DB_URL is required for CLI/scripts in README.md - Documented Unix-socket DSN rewrite behavior in model_postgres_pool_config.py Review iteration: 1/10
…ning, doc cleanup - Added :? fail-fast guard to Infisical DB_CONNECTION_URI for POSTGRES_PASSWORD (consistent with all other POSTGRES_PASSWORD refs in compose file) - Removed duplicate separator line in docker-compose.infra.yml - Clarified security comment in validate_dsn (hostname/port are intentionally included for diagnostics, only credentials are omitted) - Improved OMNIBASE_INFRA_DB_URL description in docker/README.md to clarify Docker-only vs CLI/scripts usage and password character restrictions - Added _extract_password helper to util_postgres.py that warns when peer/trust auth DSNs have empty passwords causing is_configured=False Review iteration: 2/10
…espace DSNs Include correlation_id in both _get_db_dsn() error paths (missing DSN and invalid scheme) for traceability. Normalize whitespace-only DSN values to None in PostgresConfig.from_env() to prevent misleading connection failures.
jonahgabriel
force-pushed
the
jonah/omn-2065-omnibase_infra-db-split-02-update-envexample-remove
branch
from
February 11, 2026 15:57
3a5553e to
3afab4d
Compare
The handlers conftest no longer exports POSTGRES_HOST after the DB URL migration. Registration tests use PostgresConfig.from_env() directly.
…-database path - Fixed: model_postgres_pool_config.py:142 - parsed.port raises ValueError for non-numeric ports, masking the "missing database name" error. Wrapped in try/except to produce safe diagnostic message regardless of port format. Review iteration: 1/10
…rd param - Strip whitespace from OMNIBASE_INFRA_DB_URL in plugin.py (should_activate and initialize) and commands.py (_get_db_dsn) so whitespace-only values produce a clear "not set" error instead of a confusing scheme error - Type _extract_password parsed param as ParseResult instead of object, replace getattr with direct attribute access Review iteration: 1/10
…tness - Use ipaddress.IPv6Address for definitive IPv6 detection in _format_host() instead of fragile ":" in host heuristic (config_store.py) - Add inline comments for Docker Compose fallback DSN: hardcoded host:port, POSTGRES_HOST not used, Compose v2.20+ requirement (docker-compose.infra.yml) - Document hostname lowercase normalisation and decoded-credential semantics in from_dsn() docstring (model_postgres_pool_config.py) - Replace fragile string-matching on error messages with pre-check of DSN scheme for error-code mapping (backfill_capabilities.py) - Add explicit override example for autouse _skip_materialize_dependencies fixture docstring (tests/unit/conftest.py) Review iteration: 1/10
DependencyMaterializer.__init__() was eagerly calling ModelMaterializerConfig.from_env(), which resolved ALL provider configs (postgres, kafka, http) upfront. After OMN-2065 made OMNIBASE_INFRA_DB_URL fail-fast required, this crashed 17 integration tests that exercise handler discovery without ever materializing a Postgres pool. Defer sub-config resolution to _create_resource() so env vars are only required when that specific resource type is requested. Production fail-fast behavior is preserved: a contract declaring postgres_pool still fails immediately if the URL is missing. Adds 4 regression tests locking in the lazy behavior.
jonahgabriel
deleted the
jonah/omn-2065-omnibase_infra-db-split-02-update-envexample-remove
branch
February 11, 2026 18:14
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.
Summary
POSTGRES_DATABASE=omninode_bridgereferences with per-service*_DB_URLDSN pattern (DB-SPLIT-02)OMNIBASE_INFRA_DB_URL,OMNIINTELLIGENCE_DB_URL,OMNICLAUDE_DB_URL,OMNIMEMORY_DB_URL,OMNINODE_CLOUD_DB_URL,OMNIDASH_ANALYTICS_DB_URL) to.env.examplefilesValueError/ClickExceptionwhenOMNIBASE_INFRA_DB_URLis not set (no silent fallback to legacy DB)Changes (31 files)
Source code
ModelPostgresPoolConfig.from_env()now parsesOMNIBASE_INFRA_DB_URLDSN; newfrom_dsn()classmethod_get_db_dsn(),get_postgres_dsn()) read URL directlyOMNIBASE_INFRA_DB_URLinstead ofPOSTGRES_HOSTdlq_replay.py,backfill_capabilities.py) use DB URLConfiguration
.env.example(root, docker, integration) updated with all 6 DB URLsOMNIBASE_INFRA_DB_URLfor app connections, hardcoded DB names for container init.secretresolver_allowlistcleaned upTests
OMNIBASE_INFRA_DB_URLPostgresConfig.from_env()prefers URL-based configfrom_env()behaviorDocs
Test plan
POSTGRES_DATABASEreferences remain in codebase (grepconfirms)mypyclean on all 4 modified source filesOMNIBASE_INFRA_DB_URLsetCloses OMN-2065
Summary by CodeRabbit
New Features
Chores
Documentation
Tests