refactor: just make getting value for rule_result more easy to read#1965
refactor: just make getting value for rule_result more easy to read#1965immanuwell wants to merge 1 commit intorobusta-dev:masterfrom
Conversation
WalkthroughRefactors SLA operator comparison logic in Prometheus enrichments from explicit multi-branch conditionals (>, <, ==, !=) to a dictionary-driven lookup, preserving behavior and defaulting to False for unknown operators. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 0
🧹 Nitpick comments (1)
playbooks/robusta_playbooks/prometheus_enrichments.py (1)
234-240: The dictionary-based lookup is cleaner, though it evaluates all comparisons eagerly.The refactor successfully simplifies the operator comparison logic and improves readability. However, note that the dictionary approach evaluates all four comparison expressions upfront (
query_result > params.threshold,query_result < params.threshold, etc.), whereas the original if-elif chain would have short-circuited after matching the operator. For simple numeric comparisons like this, the performance difference is negligible.Optional cleanup:
- Line 233 initializes
rule_result = Falsebut this is immediately overwritten on line 240, making the initialization redundant.- The type hint on line 234 could be more specific:
results: dict[str, bool]instead ofresults: dict.Apply this diff if you'd like to remove the redundancy:
- rule_result: bool = False results: dict = { ">": query_result > params.threshold, "<": query_result < params.threshold, "==": query_result == params.threshold, "!=": query_result != params.threshold, } - rule_result = results.get(params.operator, False) + rule_result: bool = results.get(params.operator, False)
Nothing special, just refactor way of getting value for rule_result, now it uses dict instead of if-elif
In my opinion, this is much easier to read
So please review it @aantn @moshemorad