-
Notifications
You must be signed in to change notification settings - Fork 14.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Support a dynamic minimum interval for alerts and reports #29241
feat: Support a dynamic minimum interval for alerts and reports #29241
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #29241 +/- ##
===========================================
+ Coverage 60.48% 83.73% +23.24%
===========================================
Files 1931 518 -1413
Lines 76236 37541 -38695
Branches 8568 0 -8568
===========================================
- Hits 46114 31435 -14679
+ Misses 28017 6106 -21911
+ Partials 2105 0 -2105
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
One comment I had, I wonder if we should have some parameters passed to the function, even if your examples don't need them. This way we don't need to change the signature in the future. Eg, we could pass the database, so that databases where query are more expensive would have a higher minimum interval.
@betodealmeida that makes perfect sense! I added |
(cherry picked from commit 3dadefc)
SUMMARY
This PR adds the ability to control the
ALERT_MINIMUM_INTERVAL
and/orREPORT_MINIMUM_INTERVAL
configs dynamically (rather than having it statically defined during runtime). This is useful in case the value should be dynamically changed/evaluated on demand (you might want to control it externally).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No UI changes.
TESTING INSTRUCTIONS
Added unit tests to make sure the config supports both an
integer
and acallable
.ADDITIONAL INFORMATION