disable all clickhouse queries for non-full builds #213
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 4cd8b6c in 19 seconds
More details
- Looked at
239lines of code in5files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/lib/clickhouse/evaluation-scores.ts:15
- Draft comment:
Consider using template literals for the query string to improve readability and maintainability. This will make it easier to modify the query in the future. - Reason this comment was not posted:
Confidence changes required:50%
The feature flag check is correctly implemented, but the query string construction could be improved for readability and maintainability.
Workflow ID: wflow_qevhimTkc8Res8s9
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 4cd8b6c in 1 minute and 16 seconds
More details
- Looked at
239lines of code in5files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. app-server/src/ch/evaluation_scores.rs:81
- Draft comment:
The feature flag check is correctly implemented here to prevent ClickHouse queries when the 'FullBuild' feature is not enabled. This pattern is consistently applied across the modified files. - Reason this comment was not posted:
Confidence changes required:0%
The feature flag check is correctly implemented in all files, ensuring that ClickHouse queries are only executed when the 'FullBuild' feature is enabled. This is consistent across all modified files.
Workflow ID: wflow_GdOVBHLM8xSIsg4s
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -75,6 +78,10 @@ pub async fn insert_evaluation_scores( | |||
| return Ok(()); | |||
| } | |||
|
|
|||
| if !is_feature_enabled(Feature::FullBuild) { | |||
There was a problem hiding this comment.
This feature flag check for 'FullBuild' is a duplicate of existing checks in the codebase. Consider reusing the existing pattern or refactoring to avoid duplication.
- Feature flag check in
insert_events(events.rs) - Feature flag check in
insert_span(spans.rs) - Feature flag check in
insert_labels(labels.rs) - Feature flag check in
utils(utils.rs) - Feature flag check in
consumer(consumer.rs)
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on dea16b1 in 25 seconds
More details
- Looked at
165lines of code in4files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. app-server/src/ch/events.rs:130
- Draft comment:
The feature flag check forFeature::FullBuildwas removed from this function. According to the PR description, it should be added to disable ClickHouse queries for non-full builds. Please ensure the feature flag check is present. - Reason this comment was not posted:
Comment did not seem useful.
2. app-server/src/ch/spans.rs:127
- Draft comment:
The feature flag check forFeature::FullBuildwas removed from this function. According to the PR description, it should be added to disable ClickHouse queries for non-full builds. Please ensure the feature flag check is present. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_bo2W7xQvoWovIMcr
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
disable all clickhouse queries for non-full builds (#213)
Important
Disable ClickHouse queries for non-full builds by adding feature flag checks in multiple files and functions.
Feature::FullBuildis not enabled inevaluation_scores.rs,events.rs,labels.rs,spans.rs, andevaluation-scores.ts.insert_evaluation_scores,insert_events,insert_label,insert_span, and various metric retrieval functions.evaluation_scores.rs: Added feature flag check ininsert_evaluation_scores.events.rs: Added feature flag check ininsert_eventsand metric functions.labels.rs: Added feature flag check ininsert_labelanddelete_label.spans.rs: Added feature flag check ininsert_spanand metric functions.evaluation-scores.ts: Added feature flag check ingetEvaluationTimeProgression.This description was created by
for dea16b1. It will automatically update as commits are pushed.