Modularize Alerts #1390
Modularize Alerts #1390
Conversation
New traits- - `AlertTrait`: A trait describing alerts in Parseable - `AlertManagerTrait`: A trait describing the methods for in-memory alerts structure New structs- - `ThresholdAlert`: A struct describing threshold type alerts
WalkthroughThis set of changes introduces a trait-based abstraction for alert management, defining Changes
Sequence Diagram(s)sequenceDiagram
participant HTTP Handler
participant AlertManager (trait object)
participant Alert (trait object)
participant QueryExecutor
HTTP Handler->>AlertManager: get_alert_by_id(id)
AlertManager-->>HTTP Handler: Box<dyn AlertTrait>
HTTP Handler->>Alert: eval_alert()
Alert->>QueryExecutor: execute_alert_query(query, time_range)
QueryExecutor-->>Alert: f64 result
Alert-->>HTTP Handler: (bool, f64) evaluation result
HTTP Handler->>AlertManager: update_state(id, new_state, trigger_notif)
AlertManager-->>HTTP Handler: Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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). (10)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/alerts/mod.rs (1)
518-527: Apply the same error handling improvement here.This has the same issue as the previous state update - silently ignoring when alert manager is not initialized.
🧹 Nitpick comments (2)
src/alerts/mod.rs (2)
28-28: Remove commented code instead of leaving it in.Dead code should be removed rather than commented out. Version control preserves the history if needed.
-// use once_cell::sync::Lazy;
103-113: Remove large blocks of commented code.This commented code should be removed. Version control preserves the history.
-// pub static ALERTS: Lazy<Alerts> = Lazy::new(|| { -// let (tx, rx) = mpsc::channel::<AlertTask>(10); -// let alerts = Alerts { -// alerts: RwLock::new(HashMap::new()), -// sender: tx, -// }; - -// thread::spawn(|| alert_runtime(rx)); - -// alerts -// });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/alerts/alert_types.rs(1 hunks)src/alerts/alerts_utils.rs(8 hunks)src/alerts/mod.rs(23 hunks)src/alerts/target.rs(3 hunks)src/alerts/traits.rs(1 hunks)src/handlers/http/alerts.rs(7 hunks)src/handlers/http/modal/mod.rs(2 hunks)src/handlers/http/modal/server.rs(1 hunks)src/lib.rs(1 hunks)src/prism/home/mod.rs(1 hunks)src/sync.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: de-sh
PR: parseablehq/parseable#0
File: :0-0
Timestamp: 2025-03-20T15:50:45.435Z
Learning: Pay close attention to code comments for typos and semantic clarity during reviews for the Parseable project.
src/handlers/http/modal/server.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The add_tile() function in src/handlers/http/users/dashboards.rs should use get_dashboard_by_user(dashboard_id, &user_id) instead of get_dashboard(dashboard_id) to ensure proper authorization checks when modifying a dashboard.
src/sync.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/alerts/target.rs (2)
Learnt from: nikhilsinhaparseable
PR: #1346
File: src/sync.rs:78-83
Timestamp: 2025-06-16T04:56:21.613Z
Learning: In Rust async code, #[tokio::main] is appropriate when functions are called from separate OS threads (like thread::spawn()), as it creates a new Tokio runtime for that thread. The "Cannot start a runtime from within a runtime" error only occurs when #[tokio::main] functions are called from within an existing Tokio runtime context.
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/prism/home/mod.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/handlers/http/modal/mod.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/alerts/alert_types.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/alerts/traits.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/handlers/http/alerts.rs (3)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
Learnt from: nikhilsinhaparseable
PR: #1346
File: src/handlers/http/health_check.rs:81-90
Timestamp: 2025-06-16T02:04:58.990Z
Learning: In the shutdown function in src/handlers/http/health_check.rs, the design approach is to log errors from sync operations rather than propagate them. This is intentional because the shutdown function is called on SIGTERM/SIGINT signals, and the goal is to perform best-effort cleanup (syncing pending files to object storage) while allowing the shutdown to proceed regardless of sync failures. Logging provides debugging information without blocking the shutdown process.
Learnt from: nikhilsinhaparseable
PR: #1340
File: src/query/mod.rs:64-66
Timestamp: 2025-06-18T06:39:04.775Z
Learning: In src/query/mod.rs, QUERY_SESSION_STATE and QUERY_SESSION serve different architectural purposes: QUERY_SESSION_STATE is used for stats calculation and allows dynamic registration of individual parquet files from the staging path (files created every minute), while QUERY_SESSION is used for object store queries with the global schema provider. Session contexts with schema providers don't support registering individual tables/parquets, so both session objects are necessary for their respective use cases.
src/alerts/alerts_utils.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/alerts/mod.rs (3)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
Learnt from: nikhilsinhaparseable
PR: #1340
File: src/storage/object_storage.rs:31-41
Timestamp: 2025-06-18T11:15:10.836Z
Learning: DataFusion's parquet reader defaults to using view types (Utf8View, BinaryView) when reading parquet files via the schema_force_view_types configuration (default: true). This means StringViewArray and BinaryViewArray downcasting is required when processing Arrow arrays from DataFusion parquet operations, even though these types are behind nightly feature flags.
Learnt from: nikhilsinhaparseable
PR: #1340
File: src/query/mod.rs:64-66
Timestamp: 2025-06-18T06:39:04.775Z
Learning: In src/query/mod.rs, QUERY_SESSION_STATE and QUERY_SESSION serve different architectural purposes: QUERY_SESSION_STATE is used for stats calculation and allows dynamic registration of individual parquet files from the staging path (files created every minute), while QUERY_SESSION is used for object store queries with the global schema provider. Session contexts with schema providers don't support registering individual tables/parquets, so both session objects are necessary for their respective use cases.
🧬 Code Graph Analysis (3)
src/handlers/http/modal/server.rs (5)
src/handlers/http/alerts.rs (3)
get(117-133)alerts(64-67)list_tags(220-229)src/alerts/mod.rs (2)
alerts(1631-1635)list_tags(1629-1639)src/alerts/traits.rs (1)
list_tags(72-72)src/handlers/http/users/dashboards.rs (1)
list_tags(230-233)src/users/dashboards.rs (1)
list_tags(365-375)
src/sync.rs (3)
src/alerts/alert_types.rs (1)
get_id(138-140)src/alerts/traits.rs (1)
get_id(33-33)src/alerts/alerts_utils.rs (1)
evaluate_alert(55-61)
src/alerts/mod.rs (5)
src/handlers/http/mod.rs (1)
fetch_schema(92-109)src/sync.rs (1)
alert_runtime(290-350)src/alerts/alert_types.rs (3)
from(213-229)from(233-249)get_state(170-172)src/alerts/traits.rs (12)
load(54-54)list_alerts_for_user(55-59)get_alert_by_id(60-60)update(61-61)update_state(62-67)get_state(41-41)get_state(69-69)delete(68-68)start_task(70-70)delete_task(71-71)list_tags(72-72)get_all_alerts(73-73)src/utils/mod.rs (1)
user_auth_for_query(79-88)
⏰ 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). (10)
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: coverage
🔇 Additional comments (20)
src/lib.rs (1)
55-55: LGTM! Appropriate public re-export for alert system refactor.The public re-export of
datafusionaligns well with the modularization of the alert system, enabling consumers to access DataFusion functionality needed for alert query operations.src/handlers/http/modal/server.rs (1)
258-264: LGTM! Proper route registration for alert tags listing.The addition of the
/list_tagsroute under the alerts scope with appropriate authorization aligns with the alert system modularization. The use ofAction::ListDashboardfor authorization appears consistent with similar listing operations.src/handlers/http/modal/mod.rs (2)
37-37: LGTM! Proper import for alert manager access.The addition of
get_alert_managerimport supports the new trait-based alert manager interface.
186-195: LGTM! Proper async lock handling for alert manager.The refactored alert loading logic correctly implements the new pattern:
- Ensures alert manager initialization with
get_alert_manager().await- Uses proper async lock acquisition with error handling
- Provides clear error messaging when the alert manager is not set
This approach ensures thread-safe access and proper error handling consistent with the alert system modularization.
src/sync.rs (1)
298-309: LGTM! Proper trait-based alert handling.The refactored alert runtime correctly adapts to the new trait-based alert system:
- Uses
get_id()method for proper encapsulation- Employs
clone_box()for trait object cloning- Correctly dereferences the ID and passes trait object references to evaluation functions
These changes maintain the same functionality while supporting the new polymorphic alert interface.
src/prism/home/mod.rs (1)
358-366: LGTM! Consistent async lock pattern for alert manager access.The changes properly implement the new alert manager access pattern:
- Acquires async read lock for thread-safe access
- Includes proper error handling when alert manager is not set
- Maintains the same functionality while supporting the new trait-based interface
This is consistent with the alert system modularization across the codebase.
src/alerts/alert_types.rs (1)
43-210: Well-structured threshold alert implementation!The
ThresholdAlertstruct and itsAlertTraitimplementation are comprehensive and well-designed. The validation logic properly handles mode restrictions, frequency constraints, query validation, and user authorization. The evaluation method correctly delegates to utility functions for query execution and condition evaluation.src/alerts/traits.rs (1)
29-74: Excellent trait-based abstraction!The trait design is well-thought-out with clear separation of concerns between
AlertTrait(individual alert behavior) andAlertManagerTrait(alert management). The use ofasync_trait, appropriate trait bounds (Debug + Send + Sync), and theclone_boxpattern for trait object cloning demonstrate good Rust practices.src/alerts/mod.rs (12)
42-49: LGTM! Well-structured module organization.The new module structure with separate files for traits and alert types promotes better code organization and separation of concerns.
117-123: LGTM! Proper use of trait objects for polymorphism.The change to store trait objects enables polymorphic alert handling while maintaining type safety through dynamic dispatch.
230-241: LGTM! AlertType extension is well-implemented.The new
Anomalyvariant follows the existing naming conventions and display format.
643-643: LGTM! Appropriate use of async.Converting to async is correct since the method performs async storage operations.
1122-1128: LGTM! Proper feature gating for Anomaly alerts.The validation correctly restricts Anomaly alerts to Prism mode only.
1177-1177: LGTM! Well-structured query validation refactoring.The extraction of query validation into standalone functions improves modularity. The recursive logical plan analysis correctly handles nested query structures.
Also applies to: 1289-1325
1365-1366: LGTM! Proper error handling for Arrow operations.The ArrowError variant is appropriately added with correct HTTP status mapping.
Also applies to: 1390-1390
1401-1495: LGTM! Robust trait implementation with proper error handling.The implementation correctly:
- Creates appropriate trait objects based on alert type
- Provides clear error messaging for enterprise features
- Includes retry logic for task creation
- Handles v1 to v2 migration seamlessly
1499-1511: LGTM! Proper use of trait methods.The implementation correctly uses trait methods to access alert properties in a polymorphic way.
1530-1644: LGTM! Consistent trait method implementations.All trait methods are properly implemented with correct use of trait object methods and async patterns.
1670-1711: LGTM! Proper error handling and trait usage.The implementation correctly handles the case when alert manager is not initialized and uses trait methods consistently.
75-87: Potential race condition in lazy initialization.The current implementation has a TOCTOU (Time-of-Check-Time-of-Use) race condition. Between dropping the read guard and acquiring the write guard, another thread could initialize the manager.
pub async fn get_alert_manager() -> Arc<dyn AlertManagerTrait> { let guard = ALERTS.read().await; if let Some(manager) = guard.as_ref() { manager.clone() } else { drop(guard); let mut write_guard = ALERTS.write().await; + // Re-check after acquiring write lock if write_guard.is_none() { *write_guard = Some(Arc::new(create_default_alerts_manager())); } write_guard.as_ref().unwrap().clone() } }⛔ Skipped due to learnings
Learnt from: nikhilsinhaparseable PR: parseablehq/parseable#1388 File: src/alerts/mod.rs:88-104 Timestamp: 2025-07-24T11:09:21.781Z Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/alerts/target.rs (2)
104-109: LGTM! Read lock usage is appropriate for this read-only operation.The implementation correctly uses a read lock to access the alert manager when only checking if targets are in use, which allows for better concurrency. The error handling for missing alert manager is also appropriate.
292-299: LGTM! Proper error handling replaces the previous panic.The implementation now handles the missing AlertManager case gracefully by logging an error, resetting the timeout state, and returning early instead of panicking. This prevents the spawned task from silently failing and maintains system consistency.
🧹 Nitpick comments (3)
src/alerts/alert_types.rs (3)
55-57: Clarify the comment about alert stateThe comment "for new alerts, state should be resolved" is ambiguous. Consider clarifying what "resolved" means in this context.
- // for new alerts, state should be resolved + // for new alerts, state defaults to AlertState::Resolved
106-106: Potential precision issue with float comparisonUsing
gt()for float comparison could lead to precision issues. Consider using a tolerance-based comparison or rethinking the logic.- if (notif_duration.as_secs_f64()).gt(&((eval_frequency * 60) as f64)) { + if notif_duration.as_secs() > (eval_frequency * 60) {
108-108: Fix typo in error message- "evalFrequency should be greater than target repetition interval", + "evalFrequency should be greater than target repetition interval",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/alerts/alert_types.rs(1 hunks)src/alerts/alerts_utils.rs(8 hunks)src/alerts/mod.rs(24 hunks)src/alerts/target.rs(3 hunks)src/handlers/http/alerts.rs(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/alerts/alerts_utils.rs
- src/handlers/http/alerts.rs
- src/alerts/mod.rs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: de-sh
PR: parseablehq/parseable#0
File: :0-0
Timestamp: 2025-03-20T15:50:45.435Z
Learning: Pay close attention to code comments for typos and semantic clarity during reviews for the Parseable project.
src/alerts/target.rs (4)
Learnt from: nikhilsinhaparseable
PR: #1346
File: src/handlers/http/health_check.rs:81-90
Timestamp: 2025-06-16T02:04:58.990Z
Learning: In the shutdown function in src/handlers/http/health_check.rs, the design approach is to log errors from sync operations rather than propagate them. This is intentional because the shutdown function is called on SIGTERM/SIGINT signals, and the goal is to perform best-effort cleanup (syncing pending files to object storage) while allowing the shutdown to proceed regardless of sync failures. Logging provides debugging information without blocking the shutdown process.
Learnt from: nikhilsinhaparseable
PR: #1288
File: src/handlers/http/modal/mod.rs:279-301
Timestamp: 2025-04-07T13:23:10.092Z
Learning: For critical operations like writing metadata to disk in NodeMetadata::put_on_disk(), it's preferred to let exceptions propagate (using expect/unwrap) rather than trying to recover with fallback mechanisms, as the failure indicates a fundamental system issue that needs immediate attention.
Learnt from: nikhilsinhaparseable
PR: #1346
File: src/sync.rs:78-83
Timestamp: 2025-06-16T04:56:21.613Z
Learning: In Rust async code, #[tokio::main] is appropriate when functions are called from separate OS threads (like thread::spawn()), as it creates a new Tokio runtime for that thread. The "Cannot start a runtime from within a runtime" error only occurs when #[tokio::main] functions are called from within an existing Tokio runtime context.
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/alerts/alert_types.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
⏰ 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)
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
🔇 Additional comments (1)
src/alerts/target.rs (1)
302-302: LGTM! Consistent use of acquired alert manager instance.The code correctly uses the acquired
alertsinstance instead of accessing the globalALERTSdirectly, which is consistent with the new trait-based design and maintains proper lock usage.Also applies to: 320-320
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/alerts/alert_types.rs (2)
27-27: Fix typo:AlertVerisonshould beAlertVersion- AlertConfig, AlertError, AlertState, AlertType, AlertVerison, EvalConfig, Severity, + AlertConfig, AlertError, AlertState, AlertType, AlertVersion, EvalConfig, Severity,
45-45: Fix typo:AlertVerisonshould beAlertVersion- pub version: AlertVerison, + pub version: AlertVersion,
🧹 Nitpick comments (2)
src/alerts/mod.rs (2)
28-28: Remove commented importThe
once_cell::sync::Lazyimport is no longer used and should be removed.-// use once_cell::sync::Lazy;
103-113: Remove commented old implementationThe old
ALERTSstatic implementation is no longer needed and should be removed.-// pub static ALERTS: Lazy<Alerts> = Lazy::new(|| { -// let (tx, rx) = mpsc::channel::<AlertTask>(10); -// let alerts = Alerts { -// alerts: RwLock::new(HashMap::new()), -// sender: tx, -// }; - -// thread::spawn(|| alert_runtime(rx)); - -// alerts -// });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/alerts/alert_types.rs(1 hunks)src/alerts/mod.rs(27 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: de-sh
PR: parseablehq/parseable#0
File: :0-0
Timestamp: 2025-03-20T15:50:45.435Z
Learning: Pay close attention to code comments for typos and semantic clarity during reviews for the Parseable project.
src/alerts/alert_types.rs (1)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
src/alerts/mod.rs (4)
Learnt from: nikhilsinhaparseable
PR: #1388
File: src/alerts/mod.rs:88-104
Timestamp: 2025-07-24T11:09:21.781Z
Learning: In the Parseable alert system (src/alerts/mod.rs), alert versions are server-generated and controlled via CURRENT_ALERTS_VERSION constant, not user input. The AlertVerison enum's From<&str> implementation correctly defaults unknown versions to V2 since the server only generates known versions (v1, v2). Unknown versions would only occur in exceptional cases like file corruption, making the current fallback approach appropriate.
Learnt from: nikhilsinhaparseable
PR: #1340
File: src/storage/object_storage.rs:31-41
Timestamp: 2025-06-18T11:15:10.836Z
Learning: DataFusion's parquet reader defaults to using view types (Utf8View, BinaryView) when reading parquet files via the schema_force_view_types configuration (default: true). This means StringViewArray and BinaryViewArray downcasting is required when processing Arrow arrays from DataFusion parquet operations, even though these types are behind nightly feature flags.
Learnt from: nikhilsinhaparseable
PR: #1340
File: src/query/mod.rs:64-66
Timestamp: 2025-06-18T06:39:04.775Z
Learning: In src/query/mod.rs, QUERY_SESSION_STATE and QUERY_SESSION serve different architectural purposes: QUERY_SESSION_STATE is used for stats calculation and allows dynamic registration of individual parquet files from the staging path (files created every minute), while QUERY_SESSION is used for object store queries with the global schema provider. Session contexts with schema providers don't support registering individual tables/parquets, so both session objects are necessary for their respective use cases.
Learnt from: nikhilsinhaparseable
PR: #1392
File: src/migration/stream_metadata_migration.rs:303-322
Timestamp: 2025-07-28T17:10:39.448Z
Learning: In Parseable's migration system (src/migration/stream_metadata_migration.rs), each migration function updates the metadata to the current latest format using CURRENT_OBJECT_STORE_VERSION and CURRENT_SCHEMA_VERSION constants, rather than producing incremental versions. For example, v5_v6 function produces v7 format output when these constants are set to "v7", not v6 format.
🧬 Code Graph Analysis (1)
src/alerts/mod.rs (6)
src/query/mod.rs (1)
serde_json(552-552)src/handlers/http/mod.rs (1)
fetch_schema(92-109)src/sync.rs (1)
alert_runtime(290-350)src/alerts/alert_types.rs (3)
from(213-229)from(233-249)get_state(170-172)src/alerts/traits.rs (12)
load(54-54)list_alerts_for_user(55-59)get_alert_by_id(60-60)update(61-61)update_state(62-67)get_state(41-41)get_state(69-69)delete(68-68)start_task(70-70)delete_task(71-71)list_tags(72-72)get_all_alerts(73-73)src/utils/mod.rs (1)
user_auth_for_query(79-88)
⏰ 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). (10)
- GitHub Check: Build Default x86_64-apple-darwin
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: coverage
New traits-
AlertTrait: A trait describing alerts in ParseableAlertManagerTrait: A trait describing the methods for in-memory alerts structureNew structs-
ThresholdAlert: A struct describing threshold type alertsFixes #XXXX.
Description
This PR has:
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Chores
datafusioncrate for easier access by users of this library.