Skip to content

Commit

Permalink
Do not retry failing requests with status code 401 (#408)
Browse files Browse the repository at this point in the history
- Raises NonRecoverableNetworkError when request results in 401 status code

Signed-off-by: Tor Hødnebø <[email protected]>
Signed-off-by: Tor Hødnebø <[email protected]>
  • Loading branch information
Hodnebo authored Jul 3, 2024
1 parent 93e207e commit 5d869d0
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 1 deletion.
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 401

# 3.2.0 (2024-06-06)

- Update proxy authentication (databricks/databricks-sql-python#354 by @amir-haroun)
Expand Down
8 changes: 7 additions & 1 deletion src/databricks/sql/auth/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ 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.
5. The request received a 401 response, because this can never succeed.
6. The request received a 403 response, because this can never succeed.
Q: What about OSErrors and Redirects?
Expand All @@ -341,6 +342,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 == 401:
raise NonRecoverableNetworkError(
"Received 401 - UNAUTHORIZED. Confirm your authentication credentials."
)

if status_code == 403:
raise NonRecoverableNetworkError(
"Received 403 - FORBIDDEN. Confirm your authentication credentials."
Expand Down
13 changes: 13 additions & 0 deletions tests/e2e/common/retry_test_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,16 @@ def test_403_not_retried(self):
with self.connection(extra_params=self._retry_policy) as conn:
pass
assert isinstance(cm.value.args[1], NonRecoverableNetworkError)

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

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

0 comments on commit 5d869d0

Please sign in to comment.