Skip to content

Add _retry_server_directed_only mode for Retry-After header compliance#756

Open
sd-db wants to merge 4 commits intodatabricks:mainfrom
sd-db:PECOBLR-1863/retry-server-directed-only
Open

Add _retry_server_directed_only mode for Retry-After header compliance#756
sd-db wants to merge 4 commits intodatabricks:mainfrom
sd-db:PECOBLR-1863/retry-server-directed-only

Conversation

@sd-db
Copy link
Collaborator

@sd-db sd-db commented Mar 18, 2026

Summary

  • Adds a new opt-in _retry_server_directed_only connection parameter that restricts retries to only occur when the server includes a Retry-After header in the response
  • Prevents duplicate side effects for non-idempotent ExecuteStatement operations where the server has not explicitly signaled that retry is safe
  • Threads the parameter through ClientContext, all three DatabricksRetryPolicy construction sites (Thrift, SEA, UnifiedHttpClient), and the retry policy's should_retry/is_retry methods
  • Default behavior (retry without requiring the header) is unchanged

Test plan

  • 8 new unit tests covering server-directed-only mode in tests/unit/test_retry.py
  • Fixed mock fixture in tests/unit/test_unified_http_client.py for the new attribute
  • Full unit test suite passes (poetry run python -m pytest tests/unit)

When enabled, the connector only retries on 429/503 if the server includes
a Retry-After header in the response. This prevents duplicate side effects
for non-idempotent ExecuteStatement operations where the server has not
explicitly signaled that retry is safe.

The new opt-in parameter `_retry_server_directed_only` threads through
ClientContext, all three DatabricksRetryPolicy construction sites (Thrift,
SEA, UnifiedHttpClient), and the retry policy's should_retry/is_retry
methods. Default behavior (retry without requiring the header) is unchanged.

Signed-off-by: Shubham Dhal <shubham.dhal@databricks.com>
@sd-db sd-db changed the title Add _retry_server_directed_only mode for Retry-After header compliance Add _retry_server_directed_only mode for Retry-After header compliance Mar 18, 2026
Inline kwargs.get() at the single point of use in ThriftDatabricksClient
and SeaHttpClient instead of storing as dead instance state.

Signed-off-by: Shubham Dhal <shubham.dhal@databricks.com>
- Rename server_directed_only to respect_server_retry_after_header
  throughout for clarity
- Store _respect_server_retry_after_header as instance variable in
  Thrift/SEA backends to match existing kwargs extraction pattern
- Replace duplicate test fixture with _make_retry_policy(**overrides)
  helper for flexible policy construction in tests

Signed-off-by: Shubham Dhal <shubham.dhal@databricks.com>
Copy link
Contributor

@jprakash-db jprakash-db left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for making the changes

Signed-off-by: Shubham Dhal <shubham.dhal@databricks.com>
@sd-db sd-db changed the base branch from main to release/v4.1.5 March 18, 2026 18:27
@sd-db sd-db changed the base branch from release/v4.1.5 to main March 18, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants