Skip to content

Add aggregated Postgres writer for status change topic#189

Open
kevinwallimann wants to merge 10 commits into
masterfrom
feature/status-change-aggregation
Open

Add aggregated Postgres writer for status change topic#189
kevinwallimann wants to merge 10 commits into
masterfrom
feature/status-change-aggregation

Conversation

@kevinwallimann

@kevinwallimann kevinwallimann commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Overview

  • Added a Postgres writer for the status_change topic
  • Added ADR explaining merge strategy for event aggregation, handling out-of-order and duplicate events
  • Updated test config to work with colima on macOS

Release Notes

  • Add aggregated Postgres writer for status change topic

Related

Closes #188

Summary by CodeRabbit

  • New Features

    • Added support for writing and aggregating job status-change events into Postgres.
    • New job status records now capture lifecycle timestamps, retry information, nested jobs, and additional context.
  • Bug Fixes

    • Improved handling of out-of-order and duplicate status events so stored job state stays consistent.
    • Added retry logic for database startup during integration testing, helping macOS users running Docker via colima.
  • Documentation

    • Updated setup notes with extra integration test guidance for colima-based Docker environments.

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Adds a Postgres aggregation feature for status_change events: a new ADR document describing the job table design, an upsert SQL query with merge semantics, writer dispatch logic to persist events, schema/config updates, and comprehensive integration tests.

Changes

Status Change Aggregation

Layer / File(s) Summary
ADR documentation
adr/001-status-change-db/001-status-change-db.md
Documents the job table schema, out-of-order/duplicate handling, field merge strategy, and sample query patterns for aggregating status_change events.
Upsert query and topic wiring
src/writers/sql/inserts.sql, src/utils/constants.py
Adds upsert_status_change named query implementing conditional field merging on conflict, and adds TOPIC_STATUS_CHANGE to POSTGRES_WRITE_TOPICS.
Writer dispatch and upsert logic
src/writers/writer_postgres.py
Adds _upsert_status_change, timestamp conversion, event-type-to-lifecycle-field mapping, and dispatches TOPIC_STATUS_CHANGE messages through _write_topic.
Schema and access config
tests/integration/schemas/postgres_schema.py, conf/access.json, tests/integration/conftest.py, DEVELOPER.md
Adds the aggregated job table DDL, expands topic access to IntegrationTestUser, adds Postgres connection retry logic, and documents colima Docker socket setup.
Integration tests
tests/integration/test_status_change_writer.py
Adds tests for job creation/start, finish/update ordering, nested jobs, retries, and idempotency against the aggregated job table.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant EventGate
  participant WriterPostgres
  participant Postgres

  Client->>EventGate: POST status_change event
  EventGate->>WriterPostgres: _write_topic(TOPIC_STATUS_CHANGE, event)
  WriterPostgres->>WriterPostgres: _upsert_status_change(event)
  WriterPostgres->>WriterPostgres: map event_type to created_at/started_at/finished_at
  WriterPostgres->>Postgres: execute upsert_status_change (ON CONFLICT job_id DO UPDATE)
  Postgres-->>WriterPostgres: aggregated job row upserted
  WriterPostgres-->>EventGate: commit
Loading

Possibly related PRs

  • AbsaOSS/EventGate#104: Refactors writer_postgres.py into a class-based writer flow that this PR extends with TOPIC_STATUS_CHANGE handling.
  • AbsaOSS/EventGate#155: Wires up the public.cps.za.status_change topic end-to-end, complementing the writer-side aggregation added here.

Suggested labels: enhancement

Suggested reviewers: petr-pokorny-absa, lsulak, oto-macenauer-absa

Poem

A hop, a skip, a job well begun,
Statuses gathered, merged into one,
Postgres tables hum with delight,
Timestamps aligned, both day and night,
This rabbit thumps for the ADR too —
🐇 One job, one row, aggregated anew!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding an aggregated Postgres writer for the status_change topic.
Description check ✅ Passed The description follows the template with Overview, Release Notes, and Related sections, and includes the linked issue.
Linked Issues check ✅ Passed The PR implements status_change aggregation into a Postgres table and adds the needed writer, schema, and tests for #188.
Out of Scope Changes check ✅ Passed The extra ADR, test config, schema, and integration test updates support the feature and are not obviously unrelated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/status-change-aggregation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kevinwallimann kevinwallimann force-pushed the feature/status-change-aggregation branch from a7f6b26 to 83fb752 Compare July 8, 2026 08:57
@kevinwallimann kevinwallimann marked this pull request as ready for review July 8, 2026 08:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
adr/001-status-change-db/001-status-change-db.md (1)

51-59: 🩺 Stability & Availability | 🔵 Trivial

Verify FK constraints don't break out-of-order parent-child event processing.

The ADR defines FK constraints on parent_job_id and initial_job_id, but also states events may be received out-of-order during failure scenarios. If a child job event arrives before its parent event, the FK constraint would reject the insert. The integration test schema intentionally omits these constraints, so this scenario is untested. Confirm whether parent-child event ordering is guaranteed in production, or whether the FK constraints should be deferred or removed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@adr/001-status-change-db/001-status-change-db.md` around lines 51 - 59, The
FK constraints on parent_job_id and initial_job_id in the ADR may conflict with
out-of-order event arrival, so review the intended production ordering
guarantees before keeping them as immediate constraints. Update the
schema/design in the ADR to either state that parent events always exist first,
or change the FK approach to deferred enforcement or remove the constraints;
reference the job relationship constraints and the event processing flow
described in the status-change DB design.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@adr/001-status-change-db/001-status-change-db.md`:
- Around line 51-59: The FK constraints on parent_job_id and initial_job_id in
the ADR may conflict with out-of-order event arrival, so review the intended
production ordering guarantees before keeping them as immediate constraints.
Update the schema/design in the ADR to either state that parent events always
exist first, or change the FK approach to deferred enforcement or remove the
constraints; reference the job relationship constraints and the event processing
flow described in the status-change DB design.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 548c66cc-bac2-47cc-949d-a932af83f3d8

📥 Commits

Reviewing files that changed from the base of the PR and between 7e2f168 and 83fb752.

📒 Files selected for processing (9)
  • DEVELOPER.md
  • adr/001-status-change-db/001-status-change-db.md
  • conf/access.json
  • src/utils/constants.py
  • src/writers/sql/inserts.sql
  • src/writers/writer_postgres.py
  • tests/integration/conftest.py
  • tests/integration/schemas/postgres_schema.py
  • tests/integration/test_status_change_writer.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aggregate status_change events and write to database table

1 participant