Skip to content

Commit

Permalink
Don't retry network requests that fail with code 403 (#373)
Browse files Browse the repository at this point in the history
* Don't retry requests that fail with 404

Signed-off-by: Jesse Whitehouse <jesse@whitehouse.dev>

* Fix lint error

Signed-off-by: Jesse Whitehouse <jesse@whitehouse.dev>

---------

Signed-off-by: Jesse Whitehouse <jesse@whitehouse.dev>
  • Loading branch information
susodapop authored Mar 15, 2024
1 parent c103352 commit 1d4da9a
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Release History

# x.x.x (TBD)

- Don't retry requests that fail with code 403

# 3.1.0 (2024-02-16)

- Revert retry-after behavior to be exponential backoff (#349)
Expand Down
6 changes: 6 additions & 0 deletions src/databricks/sql/auth/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
default, this means ExecuteStatement is only retried for codes 429 and 503.
This limit prevents automatically retrying non-idempotent commands that could
be destructive.
5. The request received a 403 response, because this can never succeed.
Q: What about OSErrors and Redirects?
Expand All @@ -338,6 +339,11 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
if status_code == 200:
return False, "200 codes are not retried"

if status_code == 403:
raise NonRecoverableNetworkError(
"Received 403 - FORBIDDEN. Confirm your authentication credentials."
)

# Request failed and server said NotImplemented. This isn't recoverable. Don't retry.
if status_code == 501:
raise NonRecoverableNetworkError("Received code 501 from server.")
Expand Down
14 changes: 14 additions & 0 deletions tests/e2e/common/retry_test_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -407,3 +407,17 @@ def test_retry_max_redirects_exceeds_max_attempts_count_warns_user(self, caplog)
def test_retry_legacy_behavior_warns_user(self, caplog):
with self.connection(extra_params={**self._retry_policy, "_enable_v3_retries": False}):
assert "Legacy retry behavior is enabled for this connection." in caplog.text


def test_403_not_retried(self):
"""GIVEN the server returns a code 403
WHEN the connector receives this response
THEN nothing is retried and an exception is raised
"""

# Code 403 is a Forbidden error
with mocked_server_response(status=403):
with pytest.raises(RequestError) as cm:
with self.connection(extra_params=self._retry_policy) as conn:
pass
assert isinstance(cm.value.args[1], NonRecoverableNetworkError)

0 comments on commit 1d4da9a

Please sign in to comment.