Skip to content

feat(experimentation): publish warehouse connections to Redis and apply delivery status - #8551

Open
gagantrivedi wants to merge 6 commits into
mainfrom
feat/warehouse-connections-redis
Open

gagantrivedi wants to merge 6 commits into
mainfrom
feat/warehouse-connections-redis

Conversation

@gagantrivedi

@gagantrivedi gagantrivedi commented Sep 18, 2026

Copy link
Copy Markdown
Member

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

Contributes to Flagsmith/flagsmith-analytics-pipeline#29
The warehouse-delivery service reads external connections from the ingestion Redis and leaves each connection's outcome there; this is the API's side of that contract.

  • sync_environment_ingestion publishes the connection to experimentation:environment_warehouses:<client key> before routing events to the external topic, and removes it after unrouting, so the service never sees an event it cannot place. Credentials go in as the same Fernet ciphertext the database holds.
  • The connection list and detail views read the delivery service's latest outcome for each verified external connection from the experimentation:warehouse_delivery_status hash with one HMGET and, when it failed, show errored with the reason. Nothing is saved, and a connection that failed verification keeps that result. If the ingestion Redis is down or not configured, the stored status is shown.
  • The connection's ingestion hook also fires on config and credentials changes, so a rotated password reaches Redis.

Deploy before the service starts, or it commits past the backlog with nothing to deliver to. The service must be given the same WAREHOUSE_CREDENTIALS_SECRET the API task definitions already set.

How did you test this code?

Unit tests for the service functions, the task and the hook; make lint and make typecheck pass; events catalogue regenerated.

…ly delivery status

The warehouse-delivery service reads each environment's connection from
experimentation:environment_warehouses:<client key> and leaves each
connection's outcome in the experimentation:warehouse_delivery_status
hash. sync_environment_ingestion now writes the connection, with the
credentials as the same Fernet ciphertext the database holds, before the
destination so the service never sees an event it cannot place, and
removes it after. A one-minute task copies the outcomes onto
WarehouseConnection.status and status_detail. The update hook also fires
on config and credentials changes.
@vercel

vercel Bot commented Sep 18, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
docs Ready Ready Preview Sep 18, 2026 10:35am UTC
2 Skipped Deployments
Project Deployment Actions Updated
flagsmith-frontend-preview Ignored Ignored Preview Sep 18, 2026 10:35am UTC
flagsmith-frontend-staging Ignored Ignored Preview Sep 18, 2026 10:35am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

The change moves EncryptedJSONField and warehouse credential encryption into the experimentation package. It adds a shared cached Redis client and warehouse delivery synchronisation services. Warehouse connections are published, removed, and read with delivery statuses. Tasks, model hooks, and API responses use these services. Warehouse verification calls now use the verification service. Tests and observability references are updated.

Priority: ➖ Normal

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

Merge Risk: 🟠 High · up to 11bcd

Failed or pending warehouse connections can still receive routed events, causing delivery failures. This should be fixed before merge.


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.

@github-actions github-actions Bot added api Issue related to the REST API docs Documentation updates feature New feature or request and removed docs Documentation updates labels Sep 18, 2026
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.81%. Comparing base (095addd) to head (11bcdc0).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #8551    +/-   ##
========================================
  Coverage   98.81%   98.81%            
========================================
  Files        1620     1628     +8     
  Lines       66186    66394   +208     
========================================
+ Hits        65401    65609   +208     
  Misses        785      785            

☔ 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.

… module

encrypt_warehouse_credentials and decrypt_warehouse_credentials live in
core/warehouse_credentials.py, named for the secret they are bound to.
EncryptedJSONField and the ingestion sync service both call them.
@github-actions github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Sep 18, 2026
…ryption into the app

Both are only about a warehouse connection's credentials. core/fields.py
keeps NoSSRFURLField alone. Migration 0010 now imports the field from its
new module, so the migration state matches the model and no new migration
is needed.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request labels Sep 18, 2026
@github-actions github-actions Bot added the docs Documentation updates label Sep 18, 2026
…t from the ingestion one

warehouse_delivery_sync_service.py holds what the warehouse-delivery service
reads and writes: publish_warehouse_connection, remove_warehouse_connection,
pop_warehouse_delivery_statuses. ingestion_sync_service.py is back to keys
and destinations for the ingestion server. Both use the client in
ingestion_redis.py. The old warehouse_delivery_service.py, which verifies a
connection when it is saved, is renamed warehouse_verification_service.py.
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Sep 18, 2026
…ying them into Postgres

The connection list and detail views read the delivery service's latest
outcome for each verified external connection from Redis with one HMGET
and, when it failed, show errored with the reason. Nothing is saved; a
connection that failed verification keeps that result. Redis being down or
unconfigured falls back to the stored status. The one-minute
apply_warehouse_delivery_statuses task and the Lua pop go away, and
removing a connection also forgets its outcome so nothing stale shows.
@github-actions github-actions Bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Sep 18, 2026
@gagantrivedi
gagantrivedi marked this pull request as ready for review September 18, 2026 10:39
@gagantrivedi
gagantrivedi requested review from a team as code owners September 18, 2026 10:39
@gagantrivedi
gagantrivedi requested review from matthewelwell and removed request for a team September 18, 2026 10:39
@github-actions github-actions Bot removed the feature New feature or request label Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-8551 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-api-test:pr-8551 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-8551 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-8551 Finished ✅ Results
ghcr.io/flagsmith/flagsmith:pr-8551 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-8551 Finished ✅ Results

@github-actions github-actions Bot added the feature New feature or request label Sep 18, 2026
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #20510 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  35.8 seconds
commit  11bcdc0
info  🔄 Run: #20510 (attempt 1)

🗂️ Previous results
✅ private-cloud · depot-ubuntu-latest-16 — run #20510 (attempt 1)

Playwright Test Results (private-cloud - depot-ubuntu-latest-16)

passed  3 passed

Details

stats  3 tests across 3 suites
duration  48.7 seconds
commit  11bcdc0
info  🔄 Run: #20510 (attempt 1)

✅ oss · depot-ubuntu-latest-arm-16 — run #20510 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-arm-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  38.2 seconds
commit  11bcdc0
info  🔄 Run: #20510 (attempt 1)

✅ oss · depot-ubuntu-latest-16 — run #20510 (attempt 1)

Playwright Test Results (oss - depot-ubuntu-latest-16)

passed  1 passed

Details

stats  1 test across 1 suite
duration  32.9 seconds
commit  11bcdc0
info  🔄 Run: #20510 (attempt 1)

@github-actions

Copy link
Copy Markdown
Contributor

Visual Regression

19 screenshots compared. See report for details.
View full report

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0b14d203-44bd-4373-95b3-191001a70348

📥 Commits

Reviewing files that changed from the base of the PR and between 1fd274c and 11bcdc0.

📒 Files selected for processing (26)
  • api/core/fields.py
  • api/experimentation/dataclasses.py
  • api/experimentation/fields.py
  • api/experimentation/ingestion_redis.py
  • api/experimentation/ingestion_sync_service.py
  • api/experimentation/migrations/0010_warehouse_connection_credentials_and_status_detail.py
  • api/experimentation/models.py
  • api/experimentation/services.py
  • api/experimentation/tasks.py
  • api/experimentation/views.py
  • api/experimentation/warehouse_credentials.py
  • api/experimentation/warehouse_delivery_sync_service.py
  • api/experimentation/warehouse_verification_service.py
  • api/tests/unit/core/test_fields.py
  • api/tests/unit/experimentation/conftest.py
  • api/tests/unit/experimentation/test_fields.py
  • api/tests/unit/experimentation/test_ingestion_redis.py
  • api/tests/unit/experimentation/test_ingestion_sync_service.py
  • api/tests/unit/experimentation/test_models.py
  • api/tests/unit/experimentation/test_services.py
  • api/tests/unit/experimentation/test_tasks.py
  • api/tests/unit/experimentation/test_views.py
  • api/tests/unit/experimentation/test_warehouse_credentials.py
  • api/tests/unit/experimentation/test_warehouse_delivery_sync_service.py
  • api/tests/unit/experimentation/test_warehouse_verification_service.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

environment.api_key, connection_ids=[connection.id]
)
else:
warehouse_delivery_sync_service.publish_warehouse_connection(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🏁 Script executed:

sed -n '1,120p' api/experimentation/tasks.py
sed -n '50,110p' api/experimentation/models.py
sed -n '1360,1460p' api/experimentation/services.py
rg -n 'sync_environment_ingestion|verify_clickhouse_connection|WarehouseConnection.Status|status =|CONNECTED|ERRORED' api/experimentation api/tests/unit/experimentation/test_tasks.py api/tests/unit/experimentation/test_models.py api/tests/unit/experimentation/test_services.py api/tests/unit/experimentation/test_views.py

Repository: Flagsmith/flagsmith

Length of output: 20650


Gate external routing on verification status.

sync_environment_ingestion publishes every non-Flagsmith connection and enables the external destination without checking connection.status. The create and configuration hooks schedule this task before verification. verify_clickhouse_connection saves ERRORED through save(update_fields=["status", "status_detail"]), but those fields do not trigger WarehouseConnection.sync_to_ingestion. A queued task can therefore publish a pending or errored ClickHouse connection, and a failed verification does not remove an already-published destination.

Publish only verified connections. Remove the delivery document and ingestion destination when a ClickHouse connection is pending or errored. Schedule synchronisation when verification changes the persisted status.

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

Labels

api Issue related to the REST API feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant