Skip to content

Commit

Permalink
Retry on all 429 and 503, even when missing Retry-After header (#402)
Browse files Browse the repository at this point in the history
## Changes
Change HTTP client to always retry on 429 and 503, regardless of whether
Retry-After header is present. If absent, default to 1 second, as we do
today.

Subsumes #391 and #396 while we're still discussing error architecture
in the SDK.

## Tests
Unit test to ensure that 429 and 503 work with and without Retry-After
header.
  • Loading branch information
mgyucht authored Oct 13, 2023
1 parent 18e588f commit 8ee6ff3
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
11 changes: 7 additions & 4 deletions databricks/sdk/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,7 @@ def _get_details_by_type(self, error_type) -> List[ErrorDetail]:

class ApiClient:
_cfg: Config
_RETRY_AFTER_DEFAULT: int = 1

def __init__(self, cfg: Config = None):

Expand Down Expand Up @@ -1106,11 +1107,13 @@ def _is_retryable(err: BaseException) -> Optional[str]:
return f'matched {substring}'
return None

@staticmethod
def _parse_retry_after(response: requests.Response) -> Optional[int]:
@classmethod
def _parse_retry_after(cls, response: requests.Response) -> Optional[int]:
retry_after = response.headers.get("Retry-After")
if retry_after is None:
return None
# 429 requests should include a `Retry-After` header, but if it's missing,
# we default to 1 second.
return cls._RETRY_AFTER_DEFAULT
# If the request is throttled, try parse the `Retry-After` header and sleep
# for the specified number of seconds. Note that this header can contain either
# an integer or a RFC1123 datetime string.
Expand All @@ -1123,7 +1126,7 @@ def _parse_retry_after(response: requests.Response) -> Optional[int]:
except ValueError:
logger.debug(f'Invalid Retry-After header received: {retry_after}. Defaulting to 1')
# defaulting to 1 sleep second to make self._is_retryable() simpler
return 1
return cls._RETRY_AFTER_DEFAULT

def _perform(self,
method: str,
Expand Down
9 changes: 6 additions & 3 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,13 +387,16 @@ def __getattr__(self, item):
srv.shutdown()


def test_http_retry_after_handling():
@pytest.mark.parametrize('status_code,include_retry_after',
((429, False), (429, True), (503, False), (503, True)))
def test_http_retry_after(status_code, include_retry_after):
requests = []

def inner(h: BaseHTTPRequestHandler):
if len(requests) == 0:
h.send_response(429)
h.send_header('Retry-After', '1')
h.send_response(status_code)
if include_retry_after:
h.send_header('Retry-After', '1')
h.send_header('Content-Type', 'application/json')
h.end_headers()
else:
Expand Down

0 comments on commit 8ee6ff3

Please sign in to comment.