From c2b9a1ddfeea793162fe1edbbb9966611e856ada Mon Sep 17 00:00:00 2001 From: Jothi Prakash Date: Sun, 2 Feb 2025 13:27:24 -0800 Subject: [PATCH] Fixed the retry tests failing --- tests/e2e/common/retry_test_mixins.py | 20 ++++++++++---------- tests/unit/test_retry.py | 21 ++++++++++----------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/tests/e2e/common/retry_test_mixins.py b/tests/e2e/common/retry_test_mixins.py index 942955cab..b5d01a45d 100755 --- a/tests/e2e/common/retry_test_mixins.py +++ b/tests/e2e/common/retry_test_mixins.py @@ -121,9 +121,9 @@ class PySQLRetryTestsMixin: # For testing purposes _retry_policy = { "_retry_delay_min": 0.1, - "_retry_delay_max": 5, + "_retry_delay_max": 3, "_retry_stop_after_attempts_count": 5, - "_retry_stop_after_attempts_duration": 10, + "_retry_stop_after_attempts_duration": 30, "_retry_delay_default": 0.5, } @@ -135,7 +135,7 @@ def test_retry_urllib3_settings_are_honored(self): urllib3_config = {"connect": 10, "read": 11, "redirect": 12} rp = DatabricksRetryPolicy( delay_min=0.1, - delay_max=10.0, + delay_max=3, stop_after_attempts_count=10, stop_after_attempts_duration=10.0, delay_default=1.0, @@ -174,14 +174,14 @@ def test_retry_max_count_not_exceeded(self): def test_retry_exponential_backoff(self): """GIVEN the retry policy is configured for reasonable exponential backoff WHEN the server sends nothing but 429 responses with retry-afters - THEN the connector will use those retry-afters values as delay + THEN the connector will use those retry-afters values as floor """ retry_policy = self._retry_policy.copy() retry_policy["_retry_delay_min"] = 1 time_start = time.time() with mocked_server_response( - status=429, headers={"Retry-After": "3"} + status=429, headers={"Retry-After": "8"} ) as mock_obj: with pytest.raises(RequestError) as cm: with self.connection(extra_params=retry_policy) as conn: @@ -191,14 +191,14 @@ def test_retry_exponential_backoff(self): assert isinstance(cm.value.args[1], MaxRetryDurationError) # With setting delay_min to 1, the expected retry delays should be: - # 3, 3, 3, 3 + # 8, 8, 8, 8 # The first 3 retries are allowed, the 4th retry puts the total duration over the limit - # of 10 seconds + # of 30 seconds assert mock_obj.return_value.getresponse.call_count == 4 - assert duration > 6 + assert duration > 24 - # Should be less than 7, but this is a safe margin for CI/CD slowness - assert duration < 10 + # Should be less than 26, but this is a safe margin for CI/CD slowness + assert duration < 30 def test_retry_max_duration_not_exceeded(self): """GIVEN the max attempt duration of 10 seconds diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 1e18e1f43..897a1d111 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -34,8 +34,11 @@ def test_sleep__no_retry_after(self, t_mock, retry_policy, error_history): retry_policy.history = [error_history, error_history] retry_policy.sleep(HTTPResponse(status=503)) - expected_backoff_time = self.calculate_backoff_time( - 0, retry_policy.delay_min, retry_policy.delay_max + expected_backoff_time = max( + self.calculate_backoff_time( + 0, retry_policy.delay_min, retry_policy.delay_max + ), + retry_policy.delay_max, ) t_mock.assert_called_with(expected_backoff_time) @@ -54,8 +57,11 @@ def test_sleep__no_retry_after_header__multiple_retries(self, t_mock, retry_poli expected_backoff_times = [] for attempt in range(num_attempts): expected_backoff_times.append( - self.calculate_backoff_time( - attempt, retry_policy.delay_min, retry_policy.delay_max + max( + self.calculate_backoff_time( + attempt, retry_policy.delay_min, retry_policy.delay_max + ), + retry_policy.delay_max, ) ) @@ -77,10 +83,3 @@ def test_excessive_retry_attempts_error(self, t_mock, retry_policy): retry_policy.sleep(HTTPResponse(status=503)) # Internally urllib3 calls the increment function generating a new instance for every retry retry_policy = retry_policy.increment() - - @patch("time.sleep") - def test_sleep__retry_after_present(self, t_mock, retry_policy, error_history): - retry_policy._retry_start_time = time.time() - retry_policy.history = [error_history, error_history, error_history] - retry_policy.sleep(HTTPResponse(status=503, headers={"Retry-After": "3"})) - t_mock.assert_called_with(3)