feat: add --no-debounce CLI option to disable flapping service debouncing - #173
Conversation
…cing Add new --no-debounce command line flag that disables the automatic debouncing of flapping services for debugging purposes. When enabled, services are marked offline immediately upon ServiceRemoved events instead of waiting 1 second for potential recovery. Changes: - Add no_debounce field to AppState struct - Modify ServiceRemoved event handler to check flag - Update cleanup logic to skip expired removals when disabled - Add comprehensive tests for new functionality - Update README.md and manpage with new option - Update all test calls to include new parameter
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User as rgba(60,90,180,0.5) User/CLI
participant RunTui as rgba(20,160,100,0.5) run_tui
participant App as rgba(200,80,120,0.5) AppState
participant Cleanup as rgba(180,130,40,0.5) CleanupTask
participant Registry as rgba(90,90,90,0.5) ServiceRegistry
User->>RunTui: starts with --no-debounce flag
RunTui->>App: AppState::new(user_types, no_debounce)
App->>Registry: receive ServiceRemoved event
alt no_debounce == true
App->>Registry: mark service offline immediately
else no_debounce == false
App->>App: schedule_service_removal (pending_removals)
end
Cleanup->>App: periodic cleanup tick
alt no_debounce == false
Cleanup->>App: process_expired_removals -> update metrics
else no_debounce == true
Cleanup->>App: skip debounce processing
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tui_app.rs (1)
2791-2806:⚠️ Potential issue | 🟡 MinorAdd a doc comment to
run_tui. Oh boy someone is gonna get fired today.
This is a public entry point and the signature changed, so it needs a concise///doc.As per coding guidelines: “Add doc comments to public functions with `///` syntax; include examples in doc comments when helpful; keep documentation concise and focused.”📝 Proposed fix
+/// Run the TUI event loop. +/// +/// `user_service_types` are normalized and pre-seeded; set `no_debounce` to mark +/// services offline immediately. pub async fn run_tui( user_service_types: HashSet<String>, no_debounce: bool, ) -> Result<(), Box<dyn std::error::Error>> {
🤖 Fix all issues with AI agents
In `@src/tui_app.rs`:
- Around line 1869-1875: When handling ServiceEvent::ServiceRemoved, if
state.no_debounce is true you call state.mark_service_offline(&fullname) but
never notify the UI—after mark_service_offline(&fullname) emit a
Notification::ServiceChanged (or call whatever notifier is used in this module)
so the UI repaints; leave the existing schedule_service_removal(&fullname) path
unchanged for the debounced case and ensure the Notification::ServiceChanged is
sent immediately only in the no_debounce branch.
When no_debounce is enabled, mark_service_offline() was called but no notification was sent to trigger UI repaint. This resulted in services appearing to remain online in the UI until the next async event occurred. Fix ensures Notification::ServiceChanged is sent immediately when a service is successfully marked offline in the no_debounce code path.
|
@coderabbitai review Fixed a critical issue where the UI wasn't updating when services were marked offline immediately with --no-debounce flag. Now the UI will repaint properly when services are removed in real-time. |
|
Good catch fixing the repaint. That's the kind of thing that would've driven users crazy - "Hey, I turned off debouncing and now nothing updates!" Let me review the new changes to see how you fixed it. 🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tui_app.rs (1)
2793-2808:⚠️ Potential issue | 🟡 MinorAdd a doc comment for the new public
run_tuisignature.
Public API changed; document whatno_debouncedoes. Oh boy someone is gonna get fired today.✍️ Suggested doc comment
+/// Run the TUI event loop. +/// +/// `no_debounce` disables flapping-service debouncing and marks removals offline immediately. pub async fn run_tui( user_service_types: HashSet<String>, no_debounce: bool, ) -> Result<(), Box<dyn std::error::Error>> {As per coding guidelines, Add doc comments to public functions with
///syntax; include examples in doc comments when helpful; keep documentation concise and focused.
Summary
--no-debounceCLI flag to disable automatic debouncing of flapping servicesServiceRemovedeventsChanges
--no-debounceboolean flag with help textServiceRemovedevent handler to check the flagno_debouncefield toAppStatestructTesting
Backward Compatibility
Summary by CodeRabbit
New Features
Documentation
Tests