Conversation
WalkthroughThe changes focus on code formatting and restructuring within a configuration module. Imports and expressions were reformatted for readability. In the sink construction process, sink configurations are now sorted to ensure that the Robusta sink is created last. No public interfaces or method signatures were altered. Changes
Sequence Diagram(s)sequenceDiagram
participant SinksRegistry
participant SinkConfigList
participant RobustaSinkConfigWrapper
SinksRegistry->>SinkConfigList: Retrieve all sink configs
SinkConfigList->>SinksRegistry: Return configs
SinksRegistry->>SinksRegistry: Sort configs (RobustaSinkConfigWrapper last)
SinksRegistry->>SinksRegistry: Create sinks in sorted order
✨ Finishing Touches
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. 🪧 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: 0
🧹 Nitpick comments (1)
src/robusta/model/config.py (1)
59-63: Apply the static analysis suggestion for better performance.The list comprehension formatting is good, but there's a performance optimization opportunity.
Apply this diff to improve performance:
- deleted_sink_names = [ - sink_name - for sink_name in existing_sinks.keys() - if sink_name not in new_sink_names - ] + deleted_sink_names = [ + sink_name + for sink_name in existing_sinks + if sink_name not in new_sink_names + ]🧰 Tools
🪛 Ruff (0.11.9)
61-61: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/robusta/model/config.py(6 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
src/robusta/model/config.py
61-61: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: run_tests
- GitHub Check: run_tests
- GitHub Check: Deploy docs
🔇 Additional comments (10)
src/robusta/model/config.py (10)
12-15: LGTM! Import formatting improvement.The multiline import formatting improves readability and follows Python best practices.
20-22: LGTM! Import formatting improvement.Consistent with the previous import formatting change, this improves code readability.
34-36: LGTM! Logging message formatting improvement.The multiline formatting improves readability of the warning message.
37-41: LGTM! List comprehension formatting improvement.The multiline formatting makes the logic clearer and more readable.
92-95: LGTM! Conditional expression formatting improvement.The multiline formatting makes the boolean logic clearer and easier to understand.
97-104: LGTM! Conditional and logging formatting improvements.The multiline formatting improves readability of both the conditional expression and the logging statement.
151-153: LGTM! Function call formatting improvement.The multiline formatting makes the function parameters clearer.
156-158: LGTM! Constructor call formatting improvement.The multiline formatting improves readability of the constructor call.
173-177: LGTM! List comprehension formatting improvement.The multiline formatting makes the complex list comprehension more readable and easier to understand.
71-76:Details
✅ Verification successful
Verify the sorting logic correctly prioritizes RobustaSink last.
The sorting implementation correctly ensures that
RobustaSinkConfigWrapperinstances are placed at the end of the list. The boolean keyisinstance(x, RobustaSinkConfigWrapper)returnsFalsefor non-Robusta sinks andTruefor Robusta sinks, effectively sorting them last sinceFalse < True.This addresses the PR objective of handling cluster status during crashes by ensuring status is only sent after all other sinks are successfully created.
Let me verify that this sorting behavior is consistent throughout the codebase:
🏁 Script executed:
#!/bin/bash # Description: Verify RobustaSinkConfigWrapper usage and ensure no other sorting logic conflicts # Test: Search for other places where RobustaSinkConfigWrapper is used in sorting or ordering rg -A 3 -B 3 "RobustaSinkConfigWrapper.*sort|sort.*RobustaSinkConfigWrapper" # Test: Search for other sink creation or ordering logic that might conflict ast-grep --pattern 'sorted($_, key=$_)'Length of output: 1326
Sorting logic correctly prioritizes RobustaSinkConfigWrapper last
Verified that the boolean key
isinstance(x, RobustaSinkConfigWrapper)places non-Robusta sinks (False) before Robusta sinks (True), and no other conflicting ordering logic exists elsewhere in the codebase.• File: src/robusta/model/config.py, lines 71–76
| new_sink_names = [sink_config.get_name() for sink_config in new_sinks_config] | ||
| # remove deleted sinks | ||
| deleted_sink_names = [sink_name for sink_name in existing_sinks.keys() if sink_name not in new_sink_names] | ||
| deleted_sink_names = [ |
There was a problem hiding this comment.
@RoiGlinik I don't think you can change the order of the sinks
the order of the sinks matters
At least 2 places that I can think of:
- using
stopon a sink - to the finding not continue to others - Taking a value from one sink, and adding it to others (for example, taking the alert id from OpsGenie, and putting it in a Slack sink.
Please try to think of a different solution to solve this
2 participants
sort sinks config before creating them. robusta sink to the end.
only the second commit is the change
Summary by CodeRabbit
Refactor
Bug Fixes