Skip to content

ERROR:databricks.sql.common.unified_http_client:HTTP request error: Retry request would exceed Retry policy max retry duration of 300 seconds (#709)#878

Open
peco-engineer-bot[bot] wants to merge 1 commit into
mainfrom
ai/issue-709
Open

ERROR:databricks.sql.common.unified_http_client:HTTP request error: Retry request would exceed Retry policy max retry duration of 300 seconds (#709)#878
peco-engineer-bot[bot] wants to merge 1 commit into
mainfrom
ai/issue-709

Conversation

@peco-engineer-bot

Copy link
Copy Markdown
Contributor

Summary

Automated fix for #709 — ERROR:databricks.sql.common.unified_http_client:HTTP request error: Retry request would exceed Retry policy max retry duration of 300 seconds.

Aligned ClientContext's retry defaults in src/databricks/sql/auth/common.py with the connector-wide defaults (30 attempts / 900.0s) used by the Thrift and SEA backends, which are anchored to the ODBC/JDBC drivers; the previous 5/300.0 fallback cut CloudFetch downloads (which build their DatabricksRetryPolicy from ClientContext via UnifiedHttpClient) off at exactly the 300s ceiling the reporter hit. This is an offline-only client-side retry-config artifact anchored to the reference driver defaults, so a unit test (not e2e) is appropriate — a live warehouse cannot deterministically be made to time out a cloudfetch download to observe the change.

Root cause & plan

Root cause: CloudFetch result downloads (ResultSetDownloadHandler.run -> UnifiedHttpClient.request_context) build their DatabricksRetryPolicy from ClientContext, whose retry defaults diverge from every other backend. In src/databricks/sql/auth/common.py (lines 76-81) ClientContext falls back to retry_stop_after_attempts_count=5 and retry_stop_after_attempts_duration=300.0, whereas the Thrift backend (thrift_backend.py:85-86) and the SEA http client (sea/utils/http_client.py:88-91) both default these to 30 and 900.0. Because build_client_context (utils.py:996) only fills these from the user's retry* kwargs, an unset value hits the 300s/5-attempt fallback for all UnifiedHttpClient traffic — including cloudfetch downloads. So when a cloudfetch download is slow/flaky, retries are cut off at the ClientContext-only 300s ceiling and the connector raises 'Retry request would exceed Retry policy max retry duration of 300 seconds' (exactly the number the reporter saw), giving up much sooner than the documented 900s the rest of the connector uses. Fix: align ClientContext's retry_stop_after_attempts_count / retry_stop_after_attempts_duration defaults with the connector-wide documented defaults (30 / 900.0).
Files: src/databricks/sql/auth/common.py
Planned coverage:

  • With no _retry_stop_after_attempts_duration override, the retry policy used by UnifiedHttpClient/cloudfetch must default to 900.0s (matching the Thrift and SEA backends), not 300.0s. Assert on ClientContext.retry_stop_after_attempts_duration (and/or the DatabricksRetryPolicy the UnifiedHttpClient constructs) == 900.0. (default max retry duration)
  • With no _retry_stop_after_attempts_count override, the retry policy used by UnifiedHttpClient/cloudfetch must default to 30 attempts (matching Thrift/SEA), not 5. Assert on ClientContext.retry_stop_after_attempts_count == 30. (default max retry count)
  • Regression guard: when _retry_stop_after_attempts_duration / _retry_stop_after_attempts_count are passed to connect(), build_client_context still propagates them unchanged (e.g. 120 -> 120), so the default change does not clobber explicit user configuration. (user override still honored)

Files changed

  • tests/unit/test_client_context_retry_defaults.py
  • src/databricks/sql/auth/common.py

Test plan

  • tests/unit/test_client_context_retry_defaults.py::TestClientContextRetryDefaults::test_default_max_retry_duration_is_900 — fails (red) against the original code, passes (green) after the fix
  • tests/unit/test_client_context_retry_defaults.py::TestClientContextRetryDefaults::test_default_max_retry_count_is_30 — fails (red) against the original code, passes (green) after the fix
  • tests/unit/test_client_context_retry_defaults.py::TestClientContextRetryDefaults::test_user_override_still_honored — fails (red) against the original code, passes (green) after the fix

🤖 Generated by engineer-bot (bug-fix flow) — review before merge.

…etry request would exceed Retry policy max retry duration of 300 seconds (#709)

Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com>

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No issues identified by the review bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engineer-bot Maintainer-applied gate: triggers engineer-bot (bug-fix on issue / take-over on PR).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants