Ele 5146 bugfix to disable_samples flag + update package ref#2039
Ele 5146 bugfix to disable_samples flag + update package ref#2039
Conversation
|
👋 @haritamar |
WalkthroughCast and consume stored sample data as metrics for anomaly detection: tests handler now reads Changes
Sequence Diagram(s)sequenceDiagram
participant DB as test_result_db_row
participant API as tests handler
participant Schema as ElementaryTestResultSchema
DB->>API: provide test_result_db_row (sample_data)
API->>API: cast test_result_db_row.sample_data
alt disable_samples == true
API->>API: set sample_data = None
end
API->>API: metrics := test_result_db_row.sample_data
alt anomaly_detection branch
opt metrics is list and subtype != "dimension"
API->>API: sort(metrics) by end_time
end
API->>Schema: construct result with metrics -> metrics field
else non-anomaly branch
API->>Schema: construct result (previous non-anomaly flow)
end
Schema->>API: return ElementaryTestResultSchema
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Comment |
| ) -> Optional[Union[DbtTestResultSchema, ElementaryTestResultSchema]]: | ||
| test_results: Optional[Union[DbtTestResultSchema, ElementaryTestResultSchema]] | ||
|
|
||
| sample_data = test_result_db_row.sample_data if not disable_samples else None |
There was a problem hiding this comment.
There was a bug here where the disable_samples flags also removed anomaly tests rows (metrics), which is incorrect
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
elementary/monitor/api/tests/tests.py (1)
463-474: Good fix! Correctly preserves anomaly detection metrics.The variable rename to
metricsand the absence ofdisable_sampleshandling correctly addresses the issue noted in the past review - anomaly test metrics should not be affected by thedisable_samplesflag since they represent the actual test results, not just samples.Consider adding a brief comment explaining that metrics are intentionally not affected by
disable_samples:if test_result_db_row.test_type == "anomaly_detection": + # Metrics represent the actual test results and should not be disabled metrics = test_result_db_row.sample_data
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
elementary/monitor/api/tests/tests.py(2 hunks)elementary/monitor/dbt_project/package-lock.yml(1 hunks)elementary/monitor/dbt_project/packages.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
elementary/monitor/api/tests/tests.py (2)
elementary/monitor/api/tests/schema.py (1)
ElementaryTestResultSchema(8-14)elementary/monitor/alerts/test_alert.py (2)
display_name(114-115)test_sub_type_display_name(106-107)
🔇 Additional comments (3)
elementary/monitor/api/tests/tests.py (1)
447-458: LGTM! Proper implementation of disable_samples for dbt tests.The explicit cast and conditional emptying of
sample_datacorrectly implements the feature to disable samples for dbt tests when the flag is enabled.elementary/monitor/dbt_project/package-lock.yml (1)
5-6: LGTM! Consistent with packages.yml.The revision and sha1_hash are correctly updated and consistent with the dependency changes in
packages.yml.elementary/monitor/dbt_project/packages.yml (1)
4-5: Confirm intentional use of unreleased development code.The git revision exists and is valid, but it is a post-release commit on the main branch (dated 2025-10-28) that has not been included in any stable release. The latest stable release is 0.20.1 (2025-10-09). Using this commit means your dependency is on unreleased, development code rather than a tested, production release. Verify this is intentional and acceptable for your project's stability requirements. If stability is a concern, consider pinning to the latest stable release tag (e.g.,
revision: 0.20.1) instead.
null
Summary by CodeRabbit
Bug Fixes
Chores