Skip to content

fix: add missing savings env vars to runtime-effects [OMN-7569] - #1164

Merged
jonahgabriel merged 1 commit into
mainfrom
jonah/fix-savings-env-runtime-effects
Apr 6, 2026
Merged

jonahgabriel merged 1 commit into
mainfrom
jonah/fix-savings-env-runtime-effects

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Apr 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Adds OMNIBASE_INFRA_SAVINGS_KAFKA_BOOTSTRAP_SERVERS and POSTGRES_HOST to runtime-effects in docker-compose.infra.yml
  • PR fix: add missing env vars to runtime-worker [OMN-7569] #1161 applied the same fix to runtime-worker but missed runtime-effects, which also runs the savings estimation consumer
  • Updates service catalog YAMLs for both runtime-effects and runtime-worker to stay in sync with compose

Root Cause

ConfigSavingsEstimation uses env_prefix="OMNIBASE_INFRA_SAVINGS_", so the generic KAFKA_BOOTSTRAP_SERVERS=redpanda:9092 (from x-runtime-env) is not picked up. The savings consumer fails on every startup attempt with a Pydantic validation error:

kafka_bootstrap_servers: Field required [type=missing]

This results in savings_estimates table staying at 0 rows in omnidash_analytics.

Test plan

  • Redeploy runtime on .201 with infra-up-runtime
  • Verify docker exec omninode-runtime-effects env | grep SAVINGS shows the new var
  • Check docker logs omninode-runtime-effects no longer shows ConfigSavingsEstimation errors
  • Query savings_estimates table for new rows after cost events flow

Summary by CodeRabbit

  • Chores
    • Added environment variable configurations to Docker services for Kafka and database connectivity.

…569]

PR #1161 added OMNIBASE_INFRA_SAVINGS_KAFKA_BOOTSTRAP_SERVERS and
POSTGRES_HOST to runtime-worker but missed runtime-effects. The savings
estimation consumer in runtime-effects fails on every startup with:

  ConfigSavingsEstimation validation error: kafka_bootstrap_servers
  Field required [type=missing]

This is because ConfigSavingsEstimation uses env_prefix="OMNIBASE_INFRA_SAVINGS_"
so the generic KAFKA_BOOTSTRAP_SERVERS is not picked up.

Also updates service catalog YAMLs for both runtime-effects and
runtime-worker to stay in sync with docker-compose.infra.yml.
@jonahgabriel
jonahgabriel enabled auto-merge April 6, 2026 03:43
@coderabbitai

coderabbitai Bot commented Apr 6, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a8857b9d-2695-4e33-abe7-e99d3194f92a

📥 Commits

Reviewing files that changed from the base of the PR and between b699af9 and c7386f4.

📒 Files selected for processing (3)
  • docker/catalog/services/runtime-effects.yaml
  • docker/catalog/services/runtime-worker.yaml
  • docker/docker-compose.infra.yml

📝 Walkthrough

Walkthrough

Two environment variables (OMNIBASE_INFRA_SAVINGS_KAFKA_BOOTSTRAP_SERVERS and POSTGRES_HOST) are added to Docker service configurations across three YAML files to enable Kafka and PostgreSQL connectivity for savings-estimation features.

Changes

Cohort / File(s) Summary
Docker Service Catalog
docker/catalog/services/runtime-effects.yaml, docker/catalog/services/runtime-worker.yaml
Added two hardcoded environment variables: OMNIBASE_INFRA_SAVINGS_KAFKA_BOOTSTRAP_SERVERS=redpanda:9092 and POSTGRES_HOST=postgres to enable service connectivity.
Docker Compose Infrastructure
docker/docker-compose.infra.yml
Added environment variables to the runtime-effects service for Kafka bootstrap and PostgreSQL host configuration.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

🐰 Two little vars hop into place,
Redpanda and Postgres embrace,
Effects and workers all set to go,
Configuration streams now flow! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding missing environment variables to the runtime-effects service. It is specific, clear, and directly related to the primary objective of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/fix-savings-env-runtime-effects

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

@jonahgabriel
jonahgabriel added this pull request to the merge queue Apr 6, 2026
Merged via the queue into main with commit fb308ae Apr 6, 2026
65 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/fix-savings-env-runtime-effects branch April 6, 2026 04:00
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.

1 participant