Make backwards compatible with urllib3~=1.0 [Follow up #197] (#206)

* Make retry policy backwards compatible with urllib3~=1.0.0

We already implement the equivalent of backoff_max so the behaviour will
be the same for urllib3==1.x and urllib3==2.x

We do not implement backoff jitter so the behaviour for urllib3==1.x will
NOT include backoff jitter whereas urllib3==2.x WILL include jitter.

---------

Signed-off-by: Jesse Whitehouse <[email protected]>

Author: Jesse

CHANGELOG.md

@@ -1,22 +1,26 @@
 # Release History
 
-## 2.9.x (Unreleased)
+## 2.9.4 (Unreleased)
+
+## 2.9.3 (2023-08-24)
+
+- Fix: Connections failed when urllib3~=1.0.0 is installed (#206)
 
 ## 2.9.2 (2023-08-17)
 
-- Other: Add `examples/v3_retries_query_execute.py` 
-- Other: suppress log message when `_enable_v3_retries` is not `True`
-- Other: make this connector backwards compatible with `urllib3>=1.0.0`
+- Other: Add `examples/v3_retries_query_execute.py` (#199)
+- Other: suppress log message when `_enable_v3_retries` is not `True` (#199)
+- Other: make this connector backwards compatible with `urllib3>=1.0.0` (#197)
 
 ## 2.9.1 (2023-08-11)
 
-- Other: Explicitly pin urllib3 to ^2.0.0
+- Other: Explicitly pin urllib3 to ^2.0.0 (#191)
 
 ## 2.9.0 (2023-08-10)
 
-- Replace retry handling with DatabricksRetryPolicy. This is disabled by default. To enable, set `enable_v3_retries=True` when creating `databricks.sql.client`
-- Other: Fix typo in README quick start example
-- Other: Add autospec to Client mocks and tidy up `make_request`
+- Replace retry handling with DatabricksRetryPolicy. This is disabled by default. To enable, set `enable_v3_retries=True` when creating `databricks.sql.client` (#182)
+- Other: Fix typo in README quick start example (#186)
+- Other: Add autospec to Client mocks and tidy up `make_request` (#188)
 
 ## 2.8.0 (2023-07-21)
 

src/databricks/sql/auth/retry.py

@@ -56,8 +56,7 @@ class DatabricksRetryPolicy(Retry):
         `backoff_factor`.
 
     :param delay_max:
-        Float of seconds for the maximum delay between retries. This is an alias for urllib3's
-        `backoff_max`
+        Float of seconds for the maximum delay between retries.
 
     :param stop_after_attempts_count:
         Integer maximum number of attempts that will be retried. This is an alias for urllib3's
@@ -122,7 +121,6 @@ def __init__(
             total=_attempts_remaining,
             respect_retry_after_header=True,
             backoff_factor=self.delay_min,
-            backoff_max=self.delay_max,
             allowed_methods=["POST"],
             status_forcelist=[429, 503, *self.force_dangerous_codes],
         )
@@ -212,13 +210,11 @@ def new(self, **urllib3_incremented_counters: typing.Any) -> Retry:
             allowed_methods=self.allowed_methods,
             status_forcelist=self.status_forcelist,
             backoff_factor=self.backoff_factor,  # type: ignore
-            backoff_max=self.backoff_max,  # type: ignore
             raise_on_redirect=self.raise_on_redirect,
             raise_on_status=self.raise_on_status,
             history=self.history,
             remove_headers_on_redirect=self.remove_headers_on_redirect,
             respect_retry_after_header=self.respect_retry_after_header,
-            backoff_jitter=self.backoff_jitter,  # type: ignore
         )
 
         # Update urllib3's current state to reflect the incremented counters

tests/e2e/common/retry_test_mixins.py

@@ -58,14 +58,16 @@ def _test_retry_disabled_with_message(self, error_msg_substring, exception_type)
 
 
 @contextmanager
-def mocked_server_response(status: int = 200, headers: dict = {"Retry-After": None}):
+def mocked_server_response(status: int = 200, headers: dict = {}):
     """Context manager for patching urllib3 responses"""
 
     # When mocking mocking a BaseHTTPResponse for urllib3 the mock must include
     #   1. A status code
     #   2. A headers dict
     #   3. mock.get_redirect_location() return falsy
-    mock_response = MagicMock(headers=headers, status=status)
+
+    # `msg` is included for testing when urllib3~=1.0.0 is installed
+    mock_response = MagicMock(headers=headers, msg=headers, status=status)
     mock_response.get_redirect_location.return_value = False
 
     with patch("urllib3.connectionpool.HTTPSConnectionPool._get_conn") as getconn_mock:
@@ -91,7 +93,9 @@ def mock_sequential_server_responses(responses: List[dict]):
     # Each resp should have these members:
 
     for resp in responses:
-        _mock = MagicMock(headers=resp["headers"], status=resp["status"])
+        _mock = MagicMock(
+            headers=resp["headers"], msg=resp["headers"], status=resp["status"]
+        )
         _mock.get_redirect_location.return_value = False
         mock_responses.append(_mock)
 
@@ -239,7 +243,7 @@ def test_retry_safe_execute_statement_retry_condition(self):
 
         responses = [
             {"status": 429, "headers": {"Retry-After": "1"}},
-            {"status": 503, "headers": {"Retry-After": None}},
+            {"status": 503, "headers": {}},
         ]
 
         with self.connection(
@@ -262,7 +266,7 @@ def test_retry_abort_close_session_on_404(self):
         # Second response is a 404 because the session is no longer found
         responses = [
             {"status": 502, "headers": {"Retry-After": "1"}},
-            {"status": 404, "headers": {"Retry-After": None}},
+            {"status": 404, "headers": {}},
         ]
 
         with self.connection(extra_params={**self._retry_policy}) as conn:
@@ -292,7 +296,7 @@ def test_retry_abort_close_operation_on_404(self):
         # Second response is a 404 because the session is no longer found
         responses = [
             {"status": 502, "headers": {"Retry-After": "1"}},
-            {"status": 404, "headers": {"Retry-After": None}},
+            {"status": 404, "headers": {}},
         ]
 
         with self.connection(extra_params={**self._retry_policy}) as conn: