Skip to content

feat: stateless warehouse connection test endpoint#8060

Merged
Zaimwa9 merged 4 commits into
mainfrom
feat/warehouse-connection-stateless-test
Jul 22, 2026
Merged

feat: stateless warehouse connection test endpoint#8060
Zaimwa9 merged 4 commits into
mainfrom
feat/warehouse-connection-stateless-test

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

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

Stateless test endpoint for warehouse connections, so the frontend can verify ClickHouse credentials before creating or updating a connection.

  • New POST /environments/{key}/warehouse-connections/test-warehouse-connection/ (no id): validates the create-shaped payload, runs the ClickHouse check against an unsaved connection, and returns {status, status_detail} without persisting anything. ClickHouse only; other types get a 400. Throttled under the existing warehouse_connection_write scope.
  • verify_clickhouse_connection gains a persist flag; the existing detail-scoped test endpoint is unchanged.

How did you test this code?

Parametrised view tests covering reachable, unreachable, non-ClickHouse and invalid payloads, all asserting nothing is persisted. make test, make typecheck and make lint pass.

@Zaimwa9
Zaimwa9 requested review from a team as code owners July 21, 2026 16:01
@Zaimwa9
Zaimwa9 requested review from khvn26 and removed request for a team July 21, 2026 16:01
@vercel

vercel Bot commented Jul 21, 2026

Copy link
Copy Markdown

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

3 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Jul 22, 2026 9:00am
flagsmith-frontend-preview Ignored Ignored Preview Jul 22, 2026 9:00am
flagsmith-frontend-staging Ignored Ignored Preview Jul 22, 2026 9:00am

Request Review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ab701c7a-c85f-4836-a231-19750e459800

📥 Commits

Reviewing files that changed from the base of the PR and between 34287bf and b0276c4.

📒 Files selected for processing (2)
  • api/app/settings/common.py
  • openapi.yaml

📝 Walkthrough

Walkthrough

Adds an unsaved ClickHouse connection verification option, a throttled API action for testing warehouse configurations, validation and status responses, OpenAPI schemas, endpoint tests, and updated event catalogue source references.

Estimated code review effort: 3 (Moderate) | ~20 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

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 labels Jul 21, 2026
@Zaimwa9
Zaimwa9 removed the request for review from khvn26 July 21, 2026 16:02
@Zaimwa9

Zaimwa9 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@github-actions github-actions Bot added feature New feature or request and removed docs Documentation updates labels Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Docker builds report

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

@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: Pro Plus

Run ID: e2070c5a-77bf-41fe-bfb3-33c5c6484315

📥 Commits

Reviewing files that changed from the base of the PR and between 94ac7af and 41f8652.

📒 Files selected for processing (5)
  • api/experimentation/services.py
  • api/experimentation/views.py
  • api/tests/unit/experimentation/test_views.py
  • docs/docs/deployment-self-hosting/observability/_events-catalogue.md
  • openapi.yaml

Comment thread api/experimentation/views.py
Comment thread api/experimentation/views.py Outdated
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis judgement: 🧹 Ship it, nits inside

Clean feature addition: a stateless POST .../warehouse-connections/test-warehouse-connection/ (list-scoped) lets the frontend verify ClickHouse credentials before creating or updating a connection. The verify_clickhouse_connection function gains a persist=False path that sets status on the in-memory instance without saving, and the view builds an unsaved WarehouseConnection from the validated payload and returns just {status, status_detail}. Permission enforcement, throttling, SSRF protection, and feature gating are all inherited from the existing viewset infrastructure — no gaps. Tests cover reachable, unreachable, non-ClickHouse, and invalid-payload cases, and all assert nothing is persisted.

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

🧹 Nits

  • openapi.yaml:7121 — response schema references WarehouseConnection but the endpoint returns only {status, status_detail} (see inline).
📝 Walkthrough
  • services.pyverify_clickhouse_connection gains persist: bool = True; both the success and error paths guard .save() behind the flag.
  • views.py — new test_warehouse_connection_config list-scoped action builds an unsaved WarehouseConnection from validated input and calls verify_clickhouse_connection(…, persist=False), returning a minimal status dict. Throttle list updated.
  • test_views.py — parametrised test covering four payloads (reachable, unreachable, non-ClickHouse, invalid); each asserts the expected response and that no connection is persisted. Throttle test extended.
  • _events-catalogue.md — line-number updates for three log events shifted by the new code.
  • openapi.yaml — new path entry for the list-scoped test endpoint.
🧪 How to verify
  1. POST /api/v1/environments/{key}/warehouse-connections/test-warehouse-connection/ with valid ClickHouse credentials pointing at a real instance — confirm {"status": "connected", "status_detail": null}.
  2. Same request with an unreachable host — confirm {"status": "errored", "status_detail": "Could not connect to the host."} and that no WarehouseConnection row is created.
  3. Same URL with "warehouse_type": "flagsmith" — confirm 400.
  4. Confirm non-admin users receive 403 (covered by the shared WarehouseConnectionPermission class).
  5. make test — all existing and new tests pass.

Automate: the parametrised test_test_warehouse_connection_config__payload__returns_expected_response already covers (1)–(3) synthetically.

Product take: Solid UX improvement — users can validate ClickHouse credentials at form-fill time instead of discovering typos after saving. This removes a round-trip-and-rollback friction that was the main pain point in the connection setup flow.

🧭 Assumptions & unverified claims
  • Assumed DRF's DefaultRouter correctly resolves the list-scoped test-warehouse-connection route before the detail-scoped one with the same url_path, since WarehouseConnection.id is a UUID that cannot collide with the literal path segment. Verified by the test successfully reversing the URL.

A warehouse connection that never touches the warehouse — truly the best kind of test. · reviewed at 41f8652

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.65%. Comparing base (e278535) to head (41f8652).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8060   +/-   ##
=======================================
  Coverage   98.65%   98.65%           
=======================================
  Files        1514     1514           
  Lines       60110    60133   +23     
=======================================
+ Hits        59304    59327   +23     
  Misses        806      806           

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

@github-actions

github-actions Bot commented Jul 21, 2026

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

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

passed  5 passed

Details

stats  5 tests across 4 suites
duration  39.5 seconds
commit  b0276c4
info  🔄 Run: #18616 (attempt 1)

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

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

passed  3 passed

Details

stats  3 tests across 3 suites
duration  32.3 seconds
commit  b0276c4
info  🔄 Run: #18616 (attempt 1)

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

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  45 seconds
commit  b0276c4
info  🔄 Run: #18616 (attempt 1)

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

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  33.4 seconds
commit  b0276c4
info  🔄 Run: #18616 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18612 (attempt 1)

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

passed  3 passed

Details

stats  3 tests across 3 suites
duration  38.2 seconds
commit  0062a89
info  🔄 Run: #18612 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-16 — run #18612 (attempt 1)

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

passed  3 passed

Details

stats  3 tests across 3 suites
duration  32.7 seconds
commit  0062a89
info  🔄 Run: #18612 (attempt 1)

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

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  45.1 seconds
commit  34287bf
info  🔄 Run: #18613 (attempt 1)

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

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  43.3 seconds
commit  0062a89
info  🔄 Run: #18612 (attempt 1)

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

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  41.1 seconds
commit  34287bf
info  🔄 Run: #18613 (attempt 1)

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

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  41.5 seconds
commit  0062a89
info  🔄 Run: #18612 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-16 — run #18571 (attempt 1)

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

passed  1 passed

Details

stats  1 test across 1 suite
duration  56 seconds
commit  41f8652
info  🔄 Run: #18571 (attempt 1)

✅ private-cloud · depot-ubuntu-latest-arm-16 — run #18571 (attempt 1)

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

passed  3 passed

Details

stats  3 tests across 3 suites
duration  37.8 seconds
commit  41f8652
info  🔄 Run: #18571 (attempt 1)

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Visual Regression

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

gagantrivedi
gagantrivedi previously approved these changes Jul 22, 2026
@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 Jul 22, 2026
@github-actions github-actions Bot added the docs Documentation updates label Jul 22, 2026

@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: Pro Plus

Run ID: 61009a85-468a-427c-b440-68b6901eb842

📥 Commits

Reviewing files that changed from the base of the PR and between 0062a89 and 34287bf.

📒 Files selected for processing (1)
  • openapi.yaml

Comment thread openapi.yaml Outdated
@github-actions github-actions Bot added feature New feature or request and removed feature New feature or request docs Documentation updates labels Jul 22, 2026
@Zaimwa9
Zaimwa9 merged commit 6bf7a23 into main Jul 22, 2026
34 checks passed
@Zaimwa9
Zaimwa9 deleted the feat/warehouse-connection-stateless-test branch July 22, 2026 09:36
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.

2 participants