From 9f9ec88e1722f061b4891fa6d609c8b46c058c88 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" <3815206+peco-engineer-bot[bot]@users.noreply.github.com> Date: Fri, 17 Jul 2026 01:37:59 +0000 Subject: [PATCH 1/3] =?UTF-8?q?Retry:=20sleep=5Ffor=5Fretry=20uses=20max(w?= =?UTF-8?q?ait,=20delay=5Fmax)=20=E2=80=94=20inflates=20small=20Retry-Afte?= =?UTF-8?q?r=20to=20delay=5Fmax=20(60s=20floor)=20(#860)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com> --- src/databricks/sql/auth/retry.py | 2 +- tests/unit/test_retry.py | 24 ++++++++++++++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/databricks/sql/auth/retry.py b/src/databricks/sql/auth/retry.py index 9cb29fdce..62fc60a6b 100755 --- a/src/databricks/sql/auth/retry.py +++ b/src/databricks/sql/auth/retry.py @@ -297,7 +297,7 @@ def sleep_for_retry(self, response: BaseHTTPResponse) -> bool: else: proposed_wait = self.get_backoff_time() - proposed_wait = max(proposed_wait, self.delay_max) + proposed_wait = min(proposed_wait, self.delay_max) self.check_proposed_wait(proposed_wait) logger.debug(f"Retrying after {proposed_wait} seconds") time.sleep(proposed_wait) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 7abab509d..382fe947b 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -39,14 +39,21 @@ 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 = max( - self.calculate_backoff_time( - 0, retry_policy.delay_min, retry_policy.delay_max - ), - retry_policy.delay_max, + expected_backoff_time = self.calculate_backoff_time( + 0, retry_policy.delay_min, retry_policy.delay_max ) t_mock.assert_called_with(expected_backoff_time) + @patch("time.sleep") + def test_sleep__short_retry_after_is_not_inflated(self, t_mock, retry_policy): + # A small server Retry-After must be honored as-is (delay_max is a + # ceiling, not a floor). delay_max defaults to 30 in these fixtures. + retry_policy._retry_start_time = time.time() + retry_policy.history = [] + retry_policy.sleep(HTTPResponse(status=503, headers={"Retry-After": "2"})) + + t_mock.assert_called_with(2) + @patch("time.sleep") def test_sleep__no_retry_after_header__multiple_retries(self, t_mock, retry_policy): num_attempts = retry_policy.stop_after_attempts_count @@ -62,11 +69,8 @@ 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( - max( - self.calculate_backoff_time( - attempt, retry_policy.delay_min, retry_policy.delay_max - ), - retry_policy.delay_max, + self.calculate_backoff_time( + attempt, retry_policy.delay_min, retry_policy.delay_max ) ) From b29eb9a98bd3df989f36d63e72b37ef09d083e5f Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Fri, 17 Jul 2026 01:44:08 +0000 Subject: [PATCH 2/3] ai: apply changes for #868 (1 review thread) Addresses: - #3599845693 at src/databricks/sql/auth/retry.py:300 Signed-off-by: peco-engineer-bot[bot] --- tests/unit/test_retry.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index 382fe947b..d3ee95f96 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -54,6 +54,16 @@ def test_sleep__short_retry_after_is_not_inflated(self, t_mock, retry_policy): t_mock.assert_called_with(2) + @patch("time.sleep") + def test_sleep__large_retry_after_is_capped_at_delay_max(self, t_mock, retry_policy): + # A large server Retry-After is clamped down to delay_max, which acts as + # a ceiling on the wait. delay_max defaults to 30 in these fixtures. + retry_policy._retry_start_time = time.time() + retry_policy.history = [] + retry_policy.sleep(HTTPResponse(status=503, headers={"Retry-After": "120"})) + + t_mock.assert_called_with(retry_policy.delay_max) + @patch("time.sleep") def test_sleep__no_retry_after_header__multiple_retries(self, t_mock, retry_policy): num_attempts = retry_policy.stop_after_attempts_count From 834000b508bb79380ad7ac389695859da0669f1b Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Fri, 17 Jul 2026 03:21:54 +0000 Subject: [PATCH 3/3] ai: apply changes for #868 (1 review thread) Addresses: - #3599995835 at src/databricks/sql/auth/retry.py:300 Signed-off-by: peco-engineer-bot[bot] --- src/databricks/sql/auth/retry.py | 1 - tests/unit/test_retry.py | 9 +++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/databricks/sql/auth/retry.py b/src/databricks/sql/auth/retry.py index 62fc60a6b..cf18cdfda 100755 --- a/src/databricks/sql/auth/retry.py +++ b/src/databricks/sql/auth/retry.py @@ -297,7 +297,6 @@ def sleep_for_retry(self, response: BaseHTTPResponse) -> bool: else: proposed_wait = self.get_backoff_time() - proposed_wait = min(proposed_wait, self.delay_max) self.check_proposed_wait(proposed_wait) logger.debug(f"Retrying after {proposed_wait} seconds") time.sleep(proposed_wait) diff --git a/tests/unit/test_retry.py b/tests/unit/test_retry.py index d3ee95f96..1821f3916 100644 --- a/tests/unit/test_retry.py +++ b/tests/unit/test_retry.py @@ -55,14 +55,15 @@ def test_sleep__short_retry_after_is_not_inflated(self, t_mock, retry_policy): t_mock.assert_called_with(2) @patch("time.sleep") - def test_sleep__large_retry_after_is_capped_at_delay_max(self, t_mock, retry_policy): - # A large server Retry-After is clamped down to delay_max, which acts as - # a ceiling on the wait. delay_max defaults to 30 in these fixtures. + def test_sleep__large_retry_after_is_honored_as_is(self, t_mock, retry_policy): + # A server-returned Retry-After is the source of truth and must be + # honored as-is; delay_max does not cap it. delay_max defaults to 30 in + # these fixtures, so 120 would be clamped if the ceiling still applied. retry_policy._retry_start_time = time.time() retry_policy.history = [] retry_policy.sleep(HTTPResponse(status=503, headers={"Retry-After": "120"})) - t_mock.assert_called_with(retry_policy.delay_max) + t_mock.assert_called_with(120) @patch("time.sleep") def test_sleep__no_retry_after_header__multiple_retries(self, t_mock, retry_policy):