From a50ec1f68a047f8768edbdb8b8d4f713817f12be Mon Sep 17 00:00:00 2001
From: Jesse Whitehouse <jesse@whitehouse.dev>
Date: Fri, 15 Mar 2024 12:38:26 -0400
Subject: [PATCH] Don't retry requests that fail with 404

---
 CHANGELOG.md                          |  4 ++++
 src/databricks/sql/auth/retry.py      |  4 ++++
 tests/e2e/common/retry_test_mixins.py | 14 ++++++++++++++
 3 files changed, 22 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c972ab6..607656dd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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)
diff --git a/src/databricks/sql/auth/retry.py b/src/databricks/sql/auth/retry.py
index bbdd81c0..a803b17c 100644
--- a/src/databricks/sql/auth/retry.py
+++ b/src/databricks/sql/auth/retry.py
@@ -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?
@@ -337,6 +338,9 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
         # Request succeeded. Don't retry.
         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:
diff --git a/tests/e2e/common/retry_test_mixins.py b/tests/e2e/common/retry_test_mixins.py
index 9b8efd1a..9a49870e 100644
--- a/tests/e2e/common/retry_test_mixins.py
+++ b/tests/e2e/common/retry_test_mixins.py
@@ -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)
\ No newline at end of file