Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Clone omnibase_core
run: git clone --depth=1 https://github.com/OmniNode-ai/omnibase_core.git ../omnibase_core
- name: Install uv
uses: astral-sh/setup-uv@v4
with:
Expand All @@ -28,6 +30,8 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Clone omnibase_core
run: git clone --depth=1 https://github.com/OmniNode-ai/omnibase_core.git ../omnibase_core
- name: Install uv
uses: astral-sh/setup-uv@v4
with:
Expand All @@ -43,6 +47,8 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Clone omnibase_core
run: git clone --depth=1 https://github.com/OmniNode-ai/omnibase_core.git ../omnibase_core
- name: Install uv
uses: astral-sh/setup-uv@v4
with:
Expand Down
18 changes: 18 additions & 0 deletions Dockerfile.projection
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
FROM python:3.12-slim

WORKDIR /app

# Install uv for fast dependency resolution
RUN pip install --no-cache-dir uv

# Copy dependency file first for layer caching
COPY pyproject.toml ./
COPY src/ ./src/

# Install the package with projection dependencies
RUN uv pip install --system --no-cache ".[projection]" 2>/dev/null || \
uv pip install --system --no-cache aiokafka asyncpg python-dateutil && \
uv pip install --system --no-cache -e .
Comment on lines +12 to +15

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

The .[projection] install will always fail — no such optional group exists.

The pyproject.toml declares aiokafka, asyncpg, and python-dateutil as main dependencies (not in an optional [project.optional-dependencies] group). This means:

  1. uv pip install '.[projection]' will always fail (silently, due to 2>/dev/null)
  2. The fallback path always runs, making the primary command dead code
  3. Using -e . (editable install) in a production image is unusual and increases image size
🛠️ Suggested fix: Install directly without the phantom optional group
 # Install the package with projection dependencies
-RUN uv pip install --system --no-cache ".[projection]" 2>/dev/null || \
-    uv pip install --system --no-cache aiokafka asyncpg python-dateutil && \
-    uv pip install --system --no-cache -e .
+RUN uv pip install --system --no-cache .

Alternatively, if you want a separate optional group for projection-only deployments, add to pyproject.toml:

[project.optional-dependencies]
projection = ["aiokafka>=0.12.0", "asyncpg>=0.30.0", "python-dateutil>=2.9.0"]

Then remove these from main dependencies to keep the base package lighter.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile.projection` around lines 12 - 15, The RUN line currently attempts
uv pip install ".[projection]" which always fails because no 'projection'
optional group exists and then falls back to installing individual deps and
using an editable install (-e .); fix by either (A) removing the phantom
optional group and installing the package and its real dependencies directly:
replace the failing command with a non-editable install like uv pip install
--system --no-cache . (and ensure aiokafka, asyncpg, python-dateutil are listed
in pyproject.toml main dependencies), or (B) if you want a true optional group,
add [project.optional-dependencies] projection =
["aiokafka>=0.12.0","asyncpg>=0.30.0","python-dateutil>=2.9.0"] to
pyproject.toml and then use uv pip install --system --no-cache ".[projection]"
(and remove -e . in either case to avoid editable installs in production).


# Default: overridden by docker-compose command
CMD ["python", "-m", "omnimarket"]
62 changes: 62 additions & 0 deletions docker-compose.projection.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
###############################################################################
# docker-compose.projection.yml — OMN-7610 projection nodes
#
# Deploys all 6 critical Kafka→DB projection consumers as independent
# Docker services on .201 server. Each service is a standalone Python
# process consuming from its Kafka topic(s) and projecting into the
# omnidash_analytics PostgreSQL database.
#
# Usage:
# docker compose -f docker-compose.projection.yml up -d
# docker compose -f docker-compose.projection.yml logs -f
# docker compose -f docker-compose.projection.yml down
###############################################################################

x-projection-defaults: &projection-defaults
build:
context: .
dockerfile: Dockerfile.projection
restart: unless-stopped
network_mode: host
env_file: .env
environment:
KAFKA_BROKERS: ${KAFKA_BROKERS:-192.168.86.201:19092}
OMNIDASH_ANALYTICS_DB_URL: ${OMNIDASH_ANALYTICS_DB_URL}
KAFKA_CONSUMER_GROUP: ${KAFKA_CONSUMER_GROUP:-omnimarket-projections-v1}
PYTHONUNBUFFERED: "1"
Comment on lines +22 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

OMNIDASH_ANALYTICS_DB_URL has no default — service will fail if missing from .env.

Unlike KAFKA_BROKERS which has a default, the database URL will be empty if not set, causing connection failures at startup.

🛠️ Consider adding a placeholder or documentation

Either add a default that fails clearly:

-    OMNIDASH_ANALYTICS_DB_URL: ${OMNIDASH_ANALYTICS_DB_URL}
+    OMNIDASH_ANALYTICS_DB_URL: ${OMNIDASH_ANALYTICS_DB_URL:?OMNIDASH_ANALYTICS_DB_URL must be set in .env}

Or document the required variable in the file header comments.

📝 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
environment:
KAFKA_BROKERS: ${KAFKA_BROKERS:-192.168.86.201:19092}
OMNIDASH_ANALYTICS_DB_URL: ${OMNIDASH_ANALYTICS_DB_URL}
KAFKA_CONSUMER_GROUP: ${KAFKA_CONSUMER_GROUP:-omnimarket-projections-v1}
PYTHONUNBUFFERED: "1"
environment:
KAFKA_BROKERS: ${KAFKA_BROKERS:-192.168.86.201:19092}
OMNIDASH_ANALYTICS_DB_URL: ${OMNIDASH_ANALYTICS_DB_URL:?OMNIDASH_ANALYTICS_DB_URL must be set in .env}
KAFKA_CONSUMER_GROUP: ${KAFKA_CONSUMER_GROUP:-omnimarket-projections-v1}
PYTHONUNBUFFERED: "1"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.projection.yml` around lines 22 - 26, The
OMNIDASH_ANALYTICS_DB_URL environment var in the service's environment block has
no default and will be empty if missing; update the environment entry for
OMNIDASH_ANALYTICS_DB_URL to provide a clear default placeholder (e.g., an
obviously invalid connection string) so the container fails fast with a helpful
value, or alternatively add a header comment above the environment block
documenting that OMNIDASH_ANALYTICS_DB_URL is required and must be set;
reference the OMNIDASH_ANALYTICS_DB_URL key in the environment block to
implement one of these fixes.

logging:
driver: json-file
options:
max-size: "10m"
max-file: "3"

services:
projection-session-outcome:
<<: *projection-defaults
container_name: omnimarket-projection-session-outcome
command: ["python", "-m", "omnimarket.nodes.node_projection_session_outcome.handlers.handler_session_outcome"]

projection-llm-cost:
<<: *projection-defaults
container_name: omnimarket-projection-llm-cost
command: ["python", "-m", "omnimarket.nodes.node_projection_llm_cost.handlers.handler_llm_cost"]

projection-savings:
<<: *projection-defaults
container_name: omnimarket-projection-savings
command: ["python", "-m", "omnimarket.nodes.node_projection_savings.handlers.handler_savings"]

projection-delegation:
<<: *projection-defaults
container_name: omnimarket-projection-delegation
command: ["python", "-m", "omnimarket.nodes.node_projection_delegation.handlers.handler_delegation"]

projection-baselines:
<<: *projection-defaults
container_name: omnimarket-projection-baselines
command: ["python", "-m", "omnimarket.nodes.node_projection_baselines.handlers.handler_baselines"]

projection-registration:
<<: *projection-defaults
container_name: omnimarket-projection-registration
command: ["python", "-m", "omnimarket.nodes.node_projection_registration.handlers.handler_registration"]
5 changes: 4 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ dependencies = [
"omnibase_core>=0.39.0",
"pydantic>=2.12.5,<3.0.0",
"pyyaml>=6.0.2,<7.0.0",
"aiokafka>=0.12.0",
"asyncpg>=0.30.0",
"python-dateutil>=2.9.0",
]

[project.urls]
Expand Down Expand Up @@ -105,7 +108,7 @@ ignore = [
]

[tool.ruff.lint.per-file-ignores]
"tests/**/*.py" = ["T201"]
"tests/**/*.py" = ["T201", "SLF001"]

[tool.mypy]
strict = true
Expand Down
71 changes: 71 additions & 0 deletions src/omnimarket/adapters/asyncpg_adapter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""Asyncpg implementation of DatabaseAdapter."""

from __future__ import annotations

import logging
import os
from typing import Any

import asyncpg # type: ignore[import-untyped]

logger = logging.getLogger(__name__)

DB_URL_ENV = "OMNIDASH_ANALYTICS_DB_URL"


class AsyncpgAdapter:
"""DatabaseAdapter backed by asyncpg connection pool."""

def __init__(
self, dsn: str | None = None, min_size: int = 2, max_size: int = 10
) -> None:
self._dsn = dsn or os.environ.get(DB_URL_ENV, "")
self._min_size = min_size
self._max_size = max_size
self._pool: asyncpg.Pool | None = None

async def connect(self) -> None:
if not self._dsn:
raise RuntimeError(f"{DB_URL_ENV} not set and no DSN provided")
self._pool = await asyncpg.create_pool(
self._dsn,
min_size=self._min_size,
max_size=self._max_size,
command_timeout=30,
)
logger.info(
"asyncpg pool connected (min=%d, max=%d)", self._min_size, self._max_size
)

async def execute(self, query: str, *params: Any) -> list[dict[str, Any]]:
assert self._pool is not None, "call connect() first"
async with self._pool.acquire() as conn:
rows = await conn.fetch(query, *params)
Comment on lines +40 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n src/omnimarket/adapters/asyncpg_adapter.py | sed -n '30,65p'

Repository: OmniNode-ai/omnimarket

Length of output: 1883


Replace assert pool guards with real runtime checks.

assert statements are removed when Python runs with the -O flag. If connect() is not called, a missed guard falls through to None.acquire() and raises an opaque AttributeError instead of the intended contract violation.

Suggested fix
     async def execute(self, query: str, *params: Any) -> list[dict[str, Any]]:
-        assert self._pool is not None, "call connect() first"
+        if self._pool is None:
+            raise RuntimeError("call connect() first")
         async with self._pool.acquire() as conn:
             rows = await conn.fetch(query, *params)
             return [dict(r) for r in rows]

     async def execute_many(self, query: str, params_list: list[tuple[Any, ...]]) -> None:
-        assert self._pool is not None, "call connect() first"
+        if self._pool is None:
+            raise RuntimeError("call connect() first")
         async with self._pool.acquire() as conn:
             await conn.executemany(query, params_list)

     async def fetchval(self, query: str, *params: Any) -> Any:
-        assert self._pool is not None, "call connect() first"
+        if self._pool is None:
+            raise RuntimeError("call connect() first")
         async with self._pool.acquire() as conn:
             return await conn.fetchval(query, *params)

     async def execute_in_transaction(self, queries: list[tuple[str, tuple[Any, ...]]]) -> None:
         """Execute multiple queries in a single transaction."""
-        assert self._pool is not None, "call connect() first"
+        if self._pool is None:
+            raise RuntimeError("call connect() first")
         async with self._pool.acquire() as conn:
             async with conn.transaction():
                 for query, params in queries:
                     await conn.execute(query, *params)

Also applies to: 42-45, 47-50, 52-58

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/omnimarket/adapters/asyncpg_adapter.py` around lines 36 - 39, Replace
fragile assert guards with explicit runtime checks: in asyncpg_adapter methods
that use self._pool (e.g., execute) change "assert self._pool is not None" to an
explicit if-check and raise a clear exception (for example RuntimeError or
ConnectionError) with the message "call connect() first" when self._pool is
None; apply the same change to every other public method in the class that
accesses self._pool so callers get a predictable, informative error instead of
an AttributeError under -O.

return [dict(r) for r in rows]

async def execute_many(
self, query: str, params_list: list[tuple[Any, ...]]
) -> None:
assert self._pool is not None, "call connect() first"
async with self._pool.acquire() as conn:
await conn.executemany(query, params_list)

async def fetchval(self, query: str, *params: Any) -> Any:
assert self._pool is not None, "call connect() first"
async with self._pool.acquire() as conn:
return await conn.fetchval(query, *params)

async def execute_in_transaction(
self, queries: list[tuple[str, tuple[Any, ...]]]
) -> None:
"""Execute multiple queries in a single transaction."""
assert self._pool is not None, "call connect() first"
async with self._pool.acquire() as conn, conn.transaction():
for query, params in queries:
await conn.execute(query, *params)

async def close(self) -> None:
if self._pool:
await self._pool.close()
self._pool = None
logger.info("asyncpg pool closed")
27 changes: 27 additions & 0 deletions src/omnimarket/adapters/database.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
"""DatabaseAdapter protocol for projection nodes."""

from __future__ import annotations

from typing import Any, Protocol


class DatabaseAdapter(Protocol):
"""Async database adapter for projection handlers."""

async def execute(self, query: str, *params: Any) -> list[dict[str, Any]]:
"""Execute a parameterized query and return rows as dicts."""
...

async def execute_many(
self, query: str, params_list: list[tuple[Any, ...]]
) -> None:
"""Execute a parameterized query for each set of params."""
...

async def fetchval(self, query: str, *params: Any) -> Any:
"""Execute a query and return a single value."""
...

async def close(self) -> None:
"""Close the connection pool."""
...
Empty file.
Empty file.
Loading
Loading