Improve #1084 to run rate limited error retry handler correctly #1094
Conversation
| raise error | ||
|
|
||
| state.increment_current_attempt() | ||
| state.next_attempt_requested = True |
There was a problem hiding this comment.
This should exist here but it was missing in this method (this bug affects only RateLimitedErrorRetryHandler)
| if error is None: | ||
| return False | ||
|
|
||
| if isinstance(error, URLError): |
There was a problem hiding this comment.
As HttpError is a sub-class of URLError, we should have checked if the response exists here.
| raise error | ||
|
|
||
| state.increment_current_attempt() | ||
| state.next_attempt_requested = True |
There was a problem hiding this comment.
This should exist here but it was missing in this method (this bug affects only RateLimitedErrorRetryHandler)
| data: Optional[bytes] = None, | ||
| ): | ||
| self.status_code = status_code | ||
| self.status_code = int(status_code) |
There was a problem hiding this comment.
Unrelated to the bug we are fixing here; Just to make this a bit more robust
| res, | ||
| ) | ||
|
|
||
| if logger.level <= logging.DEBUG: |
There was a problem hiding this comment.
Moved this debug logging from slack_response.py
| if resp.headers.get_content_type() == "application/gzip": | ||
| # admin.analytics.getFile | ||
| body: bytes = resp.read() | ||
| if self._logger.level <= logging.DEBUG: |
There was a problem hiding this comment.
Moved this debug logging from slack_response.py
| Raises: | ||
| SlackApiError: The request to the Slack API failed. | ||
| """ | ||
| if self._logger.level <= logging.DEBUG: |
There was a problem hiding this comment.
Moved this logging from here to base client classes to print this even for the retry patterns
Codecov Report
@@ Coverage Diff @@
## main #1094 +/- ##
==========================================
+ Coverage 86.06% 86.17% +0.11%
==========================================
Files 110 110
Lines 9936 9945 +9
==========================================
+ Hits 8551 8570 +19
+ Misses 1385 1375 -10
Continue to review full report at Codecov.
|
1 participant
Summary
This pull request fixes a few bugs in #1084 about the built-in rate limited error handlers (sync/async)
Category (place an
xin each of the[ ])/docs-src(Documents, have you run./docs.sh?)/docs-src-v2(Documents, have you run./docs-v2.sh?)/tutorial(PythOnBoardingBot tutorial)tests/integration_tests(Automated tests for this library)Requirements (place an
xin each[ ])python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.shafter making the changes.