feat: push delivery of buffered events to external ClickHouse warehouses

[gagantrivedi]

Thanks for submitting a PR! Please check the boxes below:

• I have read the Contributing Guide.
• I have added information to docs/ if required so people know about the feature.
• I have filled in the "Changes" section below.
• I have filled in the "How did you test this code" section below.

Changes

Delivers buffered event objects to customers' own ClickHouse warehouses.

• Every 10 minutes, a recurring task fans out one task per active ClickHouse connection.
• Each run streams the environment's pending S3 objects (gzipped JSONEachRow from Firehose) into the customer's ClickHouse over HTTPS via clickhouse-connect's raw_insert — no decompression or parsing in the app. clickhouse-driver cannot do this: the native protocol requires the client to build columnar blocks, which its maintainer confirms is the only way data can be sent over that protocol.
• Delivered objects move to archive/; objects the warehouse rejects (bad data) move to failed/ and the run continues; connection-level failures (auth, network, missing table) abort the run, leave everything in place, and surface on the connection's status/status_detail. A later successful run resets the status.
• A run that delivers nothing and rejects everything is treated as a failure, since that points at the table's schema rather than the objects.
• Each run is bounded by a time budget so it finishes inside its task timeout, deferring the rest of a backlog to the next tick.
• Redirects are refused: the internal-address guard validates the host being dialled, and following a redirect would let a permitted host bounce the request, and its event payload, to an address that was never checked.
• The HTTPS port is inferred from the stored native port (9440→8443, 9000→8123); the SSRF host re-check from verification is repeated before connecting.

Follow-ups

• Track external warehouse delivery per object to allow retries and prevent concurrent runs #8149 — per-object delivery tracking, for retries of transient failures and claim/lease mutual exclusion.
• Ask for the ClickHouse HTTPS port instead of inferring it from the native port #8150 — ask for the HTTPS port instead of inferring it, and move verification onto the interface delivery uses. Until then, a connection on a non-standard native port verifies healthy but cannot deliver.

Customer-facing docs (the required events DDL) are deferred until the schema-ownership question in #8150 is settled, hence the unticked docs box.

How did you test this code?

• Unit tests with moto S3 and a mocked ClickHouse boundary; 100% coverage on new code.
• The gzip passthrough, epoch-millis timestamps and the object-vs-connection error codes were verified against a local ClickHouse 24.8 with the real flagsmith_exp.events DDL.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
docs	Ready	Preview, Comment	Jul 29, 2026 9:43am

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
flagsmith-frontend-preview	Ignored	Preview	Jul 29, 2026 9:43am
flagsmith-frontend-staging	Ignored	Preview	Jul 29, 2026 9:43am

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds scheduled delivery of buffered S3 event objects to ClickHouse warehouses. The change introduces ClickHouse client validation and streaming insertion, S3 pending/archive/failed object handling, bounded delivery runs, connection status updates, Prometheus metrics, recurring tasks, dependency configuration, observability documentation, and unit tests covering delivery and failure scenarios.

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.71%. Comparing base (551ac44) to head (2942db8).

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #8107    +/-   ##
========================================
  Coverage   98.70%   98.71%            
========================================
  Files        1528     1530     +2     
  Lines       60804    61195   +391     
========================================
+ Hits        60016    60407   +391     
  Misses        788      788            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gagantrivedi]

@themis-blindfold review

[themis-blindfold]

⚖️ Themis review: 🔴 Hold the merge

The delivery pipeline itself is well built — gzip passthrough, per-object vs per-connection error split, at-least-once semantics, and a genuinely thorough test suite with moto and a mocked ClickHouse boundary. Two things need fixing before this ships. The is_internal_address re-check in delivery_client is defeated by an HTTP redirect: I reproduced clickhouse-connect following a cross-host 307 and delivering the POST body to a second host. And three failure paths misreport: a run where every object is rejected still reports success and leaves the connection connected, S3 outages are surfaced to the customer as a broken warehouse, and any ClickHouse deployment on a non-standard native port can never deliver at all.

Area	Score
🎯 Correctness	2/5
🧪 Test coverage	4/5
📐 Code quality	4/5
🚀 Product impact	4/5

🔴 Blockers

• api/experimentation/warehouse_delivery_service.py:173 — the SSRF host check is bypassable via an HTTP redirect from the customer-configured host; reproduced against clickhouse-connect 1.6.0 / urllib3 2.7.0.

🟠 Majors

• api/experimentation/services.py:902 — a run in which every object is rejected is recorded as a success, so a wrong table schema silently discards all of an environment's events.
• api/experimentation/services.py:888 — S3-side failures are attributed to the customer's warehouse.
• api/experimentation/warehouse_delivery_service.py:167 — inferring the HTTP port from the native port permanently breaks any connection on a non-standard port, after verification has already reported it healthy.

🧹 Nits

• api/experimentation/warehouse_delivery_service.py:25 — module-level logger is never used.
• api/experimentation/services.py:848 — warehouse.delivery.object_rejected is missing from the regenerated events catalogue.

📝 Walkthrough

• warehouse_delivery_service.py — new boundary module: S3 listing/moving under events/ → archive//failed/, a ClickHouse HTTP client context manager with an SSRF re-check and native→HTTP port inference, and deliver_object streaming the gzipped body straight into raw_insert.
• services.py — orchestration: iterate pending objects, split object-level rejections from connection-level failures, update the connection's status/status_detail, emit counters and structured logs.
• tasks.py — a 10-minute recurring task fans out one 9-minute-timeout task per ClickHouse connection, resolving the organisation's ingestion bucket before delegating.
• metrics.py / docs — two new counters (..._delivery_runs_total, ..._delivery_objects_total) plus regenerated metrics and events catalogues.
• pyproject.toml / uv.lock — adds clickhouse-connect>=1.6.0,<2.0.0 alongside the existing native clickhouse-driver.

🧪 How to verify

• Reproduce the redirect bypass: point a connection's config.host at a host that answers 307 with Location: http://<internal-address>/, run deliver_events_for_connection, and confirm the internal address receives the POST.
• Create the customer-side events table with a deliberately wrong column type, drop two objects in events/, run the task, and check the connection status — it currently stays connected with both objects in failed/.
• Point config.port at a value outside {9000, 9440}, verify the connection through the UI (it succeeds), then run the task and observe the connection flipping to errored every tick.
• Make s3.copy_object raise a ClientError mid-loop and confirm status_detail becomes "Delivery failed." and ..._delivery_runs_total{result="failure"} increments.
• uv run pytest tests/unit/experimentation/test_tasks.py tests/unit/experimentation/test_warehouse_delivery_service.py
• Automate: add cases for an all-rejected run and for an S3 error raised inside the loop; both currently pass through the success/describe_delivery_error paths untested.

Product take: A real capability — customers can finally get their experiment events into their own ClickHouse without building an export themselves, which is the piece that makes the external-warehouse option usable end to end. Bigger than typical work in this area. The failure-reporting gaps matter disproportionately here: a customer whose delivery is silently dropping events has no signal until they go looking for rows.

Big picture: WarehouseConnection.status now has five independent writers — mark_warehouse_pending_connection, verify_clickhouse_connection, refresh_warehouse_connection_status, and the two new mark_warehouse_delivery_* helpers — each with its own idea of which transitions are legal, and none sharing the VALID_STATUS_TRANSITIONS mechanism that Experiment uses. Verification and delivery are separate state machines writing one field, which is why a delivery run can overwrite a verification result and vice versa. The smallest useful boundary is a single owner for connection-status transitions that both paths call, rather than four more save(update_fields=["status", "status_detail"]) sites.

🧭 Assumptions & unverified claims

• The redirect reproduction used clickhouse-connect 1.6.0 and urllib3 2.7.0 installed standalone from the versions pinned in uv.lock, driving get_client against local HTTP servers; it was not run inside the Flagsmith test suite or against a real ClickHouse.
• The 30-day object-expiry claim comes from the expire-objects lifecycle rule applied at prefix "" in _create_events_bucket; I did not verify it against a live bucket.
• I did not verify how the task processor enforces timeout=timedelta(minutes=9), so I have not assessed whether two runs for the same connection can overlap across the 10-minute tick.
• CI was still running at the time of review; the completed checks (patch coverage, CodeQL) had passed and the build/e2e jobs were skipped.

Impeccable plumbing for the happy path; the unhappy paths are where the events go quietly missing. · reviewed at 1172924

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

api/experimentation/services.py (2)

930-948: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not classify a budget-limited partial run as “every object rejected”.

If the first attempted object is rejected and the time budget expires before the remaining pending keys are processed, delivered_count == 0 and rejected_count still enters this branch. The connection is then marked errored with schema-mismatch guidance even though unattempted objects remain. Only apply this outcome when all pending objects were processed and rejected.

---

852-879: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make the insert-and-archive transition retry-safe.

If ClickHouse accepts the object but the subsequent S3 archive move fails, the task fails before deliver_warehouse_events() returns while the object remains under the pending prefix. The next run can deliver it again, and duplicate rows are only accounted for downstream rather than prevented at the delivery boundary. Add an idempotency mechanism or per-object delivery state before relying on this retry path, and cover archive-move failure in a regression test.

api/tests/unit/experimentation/test_tasks.py (1)

1036-1071: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert the S3 outcome in the all-rejected test.

This test verifies the connection status and log, but not that every rejected key leaves pending/ and lands under failed/. Add those assertions so the regression test also protects the object lifecycle.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e5e742d3-b55a-4931-8b99-cf8349858173

📥 Commits

Reviewing files that changed from the base of the PR and between 25148a7 and 8d201d5.

📒 Files selected for processing (2)

• api/experimentation/services.py
• api/tests/unit/experimentation/test_tasks.py

[Zaimwa9]

Approving with couple of nits and questions

[coderabbitai]

Actionable comments posted: 5

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 055b556b-b586-4aa5-af6e-37eddd3ddf98

📥 Commits

Reviewing files that changed from the base of the PR and between b1ef98f and eacbf2d.

⛔ Files ignored due to path filters (1)

• api/uv.lock is excluded by !**/*.lock

📒 Files selected for processing (9)

• api/experimentation/constants.py
• api/experimentation/metrics.py
• api/experimentation/services.py
• api/experimentation/tasks.py
• api/experimentation/warehouse_delivery_service.py
• api/pyproject.toml
• api/tests/unit/experimentation/test_tasks.py
• api/tests/unit/experimentation/test_warehouse_delivery_service.py
• docs/docs/deployment-self-hosting/observability/_metrics-catalogue.md