-
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
chore: Lower giveup log level for retried functions to warning #26188
chore: Lower giveup log level for retried functions to warning #26188
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26188 +/- ##
==========================================
+ Coverage 69.15% 69.18% +0.03%
==========================================
Files 1944 1944
Lines 75925 75927 +2
Branches 8451 8451
==========================================
+ Hits 52505 52533 +28
+ Misses 21235 21209 -26
Partials 2185 2185
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -26,13 +27,15 @@ def retry_call( | |||
*args: Any, | |||
strategy: Callable[..., Generator[int, None, None]] = backoff.constant, | |||
exception: type[Exception] = Exception, | |||
giveup_log_level: int = logging.WARNING, |
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.
maybe type this as optional?
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.
I think just int
is more precise here, since the default logging.WARNING
is an int, and it wouldn't make sense to pass giveup_log_level=None
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.
Approved, with a suggested small nit on typing.
(cherry picked from commit bf5b18c)
SUMMARY
The goal of this PR is to eliminate unnecessary error logs. The
retry_call
function uses thebackoff
lib to decorate functions we want to retry. After the retry limit is reached, thebackoff
decorator generates a log saying that it's giving up. This log defaults to the error level, which isn't necessary here since the exception raised by the wrapped function gets bubbled up and logged at the appropriate level already.This PR solves the issue by lowering the log from the
backoff
decorator to a warningBEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION