Backport: Add _retry_server_directed_only mode for Retry-After header compliance (#757)

* Add _retry_server_directed_only mode for Retry-After header compliance

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>

* Remove unnecessary _retry_server_directed_only instance variables

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>

* Address PR feedback: rename and clean up retry-after parameter

- 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>

* Fix Black formatting for retry.py and utils.py

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

* Apply CI workflow and test fixes from PR #745

- Update actions/checkout@v2 → v4 and actions/setup-python@v2 → v5 in run-unit-tests-with-arrow job
- Pin Poetry version to 2.2.1 across all workflow files
- Reduce long-running query test min duration from 3 to 2 minutes

Co-authored-by: Isaac

* Removed unnecessary tests

* Fixed something

* fixed something

---------

Signed-off-by: Shubham Dhal <shubham.dhal@databricks.com>
Co-authored-by: Jothi Prakash <jothi.prakash@databricks.com>

Author: sd-db

.github/workflows/code-quality-checks.yml

@@ -35,6 +35,7 @@ jobs:
       - name: Install Poetry
         uses: snok/install-poetry@v1
         with:
+          version: "2.2.1"
           virtualenvs-create: true
           virtualenvs-in-project: true
           installer-parallel: true
@@ -106,10 +107,10 @@ jobs:
       #       check-out repo and set-up python
       #----------------------------------------------
       -   name: Check out repository
-          uses: actions/checkout@v2
+          uses: actions/checkout@v4
       -   name: Set up python ${{ matrix.python-version }}
           id: setup-python
-          uses: actions/setup-python@v2
+          uses: actions/setup-python@v5
           with:
             python-version: ${{ matrix.python-version }}
       #----------------------------------------------
@@ -118,6 +119,7 @@ jobs:
       -   name: Install Poetry
           uses: snok/install-poetry@v1
           with:
+            version: "2.2.1"
             virtualenvs-create: true
             virtualenvs-in-project: true
             installer-parallel: true
@@ -191,6 +193,7 @@ jobs:
       - name: Install Poetry
         uses: snok/install-poetry@v1
         with:
+          version: "2.2.1"
           virtualenvs-create: true
           virtualenvs-in-project: true
           installer-parallel: true
@@ -243,6 +246,7 @@ jobs:
       - name: Install Poetry
         uses: snok/install-poetry@v1
         with:
+          version: "2.2.1"
           virtualenvs-create: true
           virtualenvs-in-project: true
           installer-parallel: true

.github/workflows/integration.yml

@@ -33,6 +33,7 @@ jobs:
       - name: Install Poetry
         uses: snok/install-poetry@v1
         with:
+          version: "2.2.1"
           virtualenvs-create: true
           virtualenvs-in-project: true
           installer-parallel: true

.github/workflows/publish-test.yml

@@ -21,6 +21,7 @@ jobs:
       - name: Install Poetry
         uses: snok/install-poetry@v1
         with:
+          version: "2.2.1"
           virtualenvs-create: true
           virtualenvs-in-project: true
           installer-parallel: true

.github/workflows/publish.yml

@@ -23,6 +23,7 @@ jobs:
       - name: Install Poetry
         uses: snok/install-poetry@v1
         with:
+          version: "2.2.1"
           virtualenvs-create: true
           virtualenvs-in-project: true
           installer-parallel: true

src/databricks/sql/auth/common.py

@@ -47,6 +47,7 @@ def __init__(
         retry_stop_after_attempts_duration: Optional[float] = None,
         retry_delay_default: Optional[float] = None,
         retry_dangerous_codes: Optional[List[int]] = None,
+        respect_server_retry_after_header: Optional[bool] = None,
         proxy_auth_method: Optional[str] = None,
         pool_connections: Optional[int] = None,
         pool_maxsize: Optional[int] = None,
@@ -79,6 +80,7 @@ def __init__(
         )
         self.retry_delay_default = retry_delay_default or 5.0
         self.retry_dangerous_codes = retry_dangerous_codes or []
+        self.respect_server_retry_after_header = bool(respect_server_retry_after_header)
         self.proxy_auth_method = proxy_auth_method
         self.pool_connections = pool_connections or 10
         self.pool_maxsize = pool_maxsize or 20

src/databricks/sql/auth/retry.py

@@ -94,6 +94,7 @@ def __init__(
         stop_after_attempts_duration: float,
         delay_default: float,
         force_dangerous_codes: List[int],
+        respect_server_retry_after_header: bool = False,
         urllib3_kwargs: dict = {},
     ):
         # These values do not change from one command to the next
@@ -103,6 +104,7 @@ def __init__(
         self.stop_after_attempts_duration = stop_after_attempts_duration
         self._delay_default = delay_default
         self.force_dangerous_codes = force_dangerous_codes
+        self.respect_server_retry_after_header = respect_server_retry_after_header
 
         # the urllib3 kwargs are a mix of configuration (some of which we override)
         # and counters like `total` or `connect` which may change between successive retries
@@ -202,6 +204,7 @@ def new(
             stop_after_attempts_duration=self.stop_after_attempts_duration,
             delay_default=self.delay_default,
             force_dangerous_codes=self.force_dangerous_codes,
+            respect_server_retry_after_header=self.respect_server_retry_after_header,
             urllib3_kwargs={},
         )
 
@@ -323,7 +326,9 @@ def get_backoff_time(self) -> float:
 
         return proposed_backoff
 
-    def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
+    def should_retry(
+        self, method: str, status_code: int, has_retry_after: bool = False
+    ) -> Tuple[bool, str]:
         """This method encapsulates the connector's approach to retries.
 
         We always retry a request unless one of these conditions is met:
@@ -381,6 +386,15 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
         if not self._is_method_retryable(method):
             return False, "Only POST requests are retried"
 
+        # When respect_server_retry_after_header is enabled, only retry when the
+        # server explicitly signals it's safe via a Retry-After header. This prevents
+        # duplicate side effects for non-idempotent operations.
+        if self.respect_server_retry_after_header and not has_retry_after:
+            return (
+                False,
+                "respect_server_retry_after_header mode: no Retry-After header present",
+            )
+
         # Request failed with 404 and was a GetOperationStatus. This is not recoverable. Don't retry.
         if status_code == 404 and self.command_type == CommandType.GET_OPERATION_STATUS:
             return (
@@ -450,7 +464,7 @@ def is_retry(
         Logs a debug message if the request will be retried
         """
 
-        should_retry, msg = self.should_retry(method, status_code)
+        should_retry, msg = self.should_retry(method, status_code, has_retry_after)
 
         if should_retry:
             logger.debug(msg)

src/databricks/sql/backend/sea/utils/http_client.py

@@ -90,6 +90,9 @@ def __init__(
         )
         self._retry_delay_default = kwargs.get("_retry_delay_default", 5.0)
         self.force_dangerous_codes = kwargs.get("_retry_dangerous_codes", [])
+        self._respect_server_retry_after_header = kwargs.get(
+            "_respect_server_retry_after_header", False
+        )
 
         # Connection pooling settings
         self.max_connections = kwargs.get("max_connections", 10)
@@ -114,6 +117,7 @@ def __init__(
                 stop_after_attempts_duration=self._retry_stop_after_attempts_duration,
                 delay_default=self._retry_delay_default,
                 force_dangerous_codes=self.force_dangerous_codes,
+                respect_server_retry_after_header=self._respect_server_retry_after_header,
                 urllib3_kwargs=urllib3_kwargs,
             )
         else:

src/databricks/sql/backend/thrift_backend.py

@@ -189,6 +189,9 @@ def __init__(
                 " This behaviour is deprecated and will be removed in a future release."
             )
         self.force_dangerous_codes = kwargs.get("_retry_dangerous_codes", [])
+        self._respect_server_retry_after_header = kwargs.get(
+            "_respect_server_retry_after_header", False
+        )
 
         additional_transport_args = {}
 
@@ -215,6 +218,7 @@ def __init__(
                 stop_after_attempts_duration=self._retry_stop_after_attempts_duration,
                 delay_default=self._retry_delay_default,
                 force_dangerous_codes=self.force_dangerous_codes,
+                respect_server_retry_after_header=self._respect_server_retry_after_header,
                 urllib3_kwargs=urllib3_kwargs,
             )
 

src/databricks/sql/common/unified_http_client.py

@@ -99,6 +99,7 @@ def _setup_pool_managers(self):
             stop_after_attempts_duration=self.config.retry_stop_after_attempts_duration,
             delay_default=self.config.retry_delay_default,
             force_dangerous_codes=self.config.retry_dangerous_codes,
+            respect_server_retry_after_header=self.config.respect_server_retry_after_header,
         )
 
         # Initialize the required attributes that DatabricksRetryPolicy expects

src/databricks/sql/utils.py

@@ -919,6 +919,9 @@ def build_client_context(server_hostname: str, version: str, **kwargs):
         ),
         retry_delay_default=kwargs.get("_retry_delay_default"),
         retry_dangerous_codes=kwargs.get("_retry_dangerous_codes"),
+        respect_server_retry_after_header=kwargs.get(
+            "_respect_server_retry_after_header"
+        ),
         proxy_auth_method=kwargs.get("_proxy_auth_method"),
         pool_connections=kwargs.get("_pool_connections"),
         pool_maxsize=kwargs.get("_pool_maxsize"),