feat: expose distinct warehouse event names per connection - #8231
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded 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. Comment |
Docker builds report
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c52d9b3b-e0b8-4d2e-b9e1-d6a267aaad7e
📒 Files selected for processing (6)
api/experimentation/dataclasses.pyapi/experimentation/services.pyapi/experimentation/views.pyapi/tests/unit/experimentation/test_services.pyapi/tests/unit/experimentation/test_views.pydocs/docs/deployment-self-hosting/observability/_events-catalogue.md
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0677e53-25d8-4fc2-8cab-b5f948dcfe83
📒 Files selected for processing (2)
docs/docs/deployment-self-hosting/observability/_events-catalogue.mdopenapi.yaml
✅ private-cloud · depot-ubuntu-latest-16 — run #19077 (attempt 2)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
🗂️ Previous results❌ private-cloud · depot-ubuntu-latest-16 — run #19077 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Failed testsfirefox › tests/environment-permission-test.pw.ts › Environment Permission Tests › Environment-level permissions control access to features, identities, and segments @enterprise ✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19077 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19077 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19077 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19054 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19054 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19054 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19055 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19054 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19055 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19052 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8231 +/- ##
========================================
Coverage 98.72% 98.72%
========================================
Files 1543 1543
Lines 61683 61805 +122
========================================
+ Hits 60895 61017 +122
Misses 788 788 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/experimentation/views.py (1)
231-241: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDeclare the 400 and 503 responses for
warehouse_connections/{connection_id}/events.The endpoint can return
{"detail": "Event listing is not supported for this warehouse type."}whenwarehouse_typeis unsupported, and{"detail": "The warehouse is currently unreachable."}when event names are unavailable. Update@extend_schemawith 400 and 503 responses and regenerateopenapi.yaml.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 22173458-6db0-4456-b53c-6192d3537e31
📒 Files selected for processing (3)
api/experimentation/services.pyapi/experimentation/views.pyapi/tests/unit/experimentation/test_views.py
|
@themis-blindfold review |
⚖️ Themis review: ✅ Ship itSolid addition: a new read endpoint for warehouse event names, properly gated behind the existing feature flag and permission class, with per-connection caching for BYO ClickHouse and cache invalidation wired into the update path. The dispatch logic (managed vs. customer ClickHouse) mirrors the existing event stats pattern, the truncation detection via LIMIT+1 is clean, and the test matrix covers both warehouse types, failure caching, truncation, and cache clearing on credential changes.
📝 Walkthrough
🧪 How to verify
Automate: steps 1–5 are covered by the existing parametrised unit tests; a staging smoke test against a real BYO ClickHouse would add confidence. Product take: This unlocks event-name selection in the metric creation UI, replacing a manual entry step with a dropdown sourced from the warehouse. Solid improvement for the experimentation workflow — not a capability expansion, but a meaningful UX upgrade that reduces setup friction and typo-driven misconfiguration. 🧭 Assumptions & unverified claims
A cache that knows when to forget is worth more than one that never does · reviewed at 2b3617c |
docs/if required so people know about the feature.Changes
Adds
GET /environments/{key}/warehouse-connections/{id}/events/, returning the distinct event names recorded in the connection's warehouse so metric creation can offer them for selection.delivery_clientwith a 5-minute per-connection cache (successes and failures).is_truncatedflag as the hook for future pagination.503when unreachable,400for unsupported types; read throttle scope applies.How did you test this code?
Parametrised unit tests (dispatch, truncation, caching, response mapping); manual check against a ClickHouse Cloud instance via a BYO connection.