Skip to content

Commit ca25c61

Browse files
committed
keep it simple :)
1 parent 65e8587 commit ca25c61

File tree

4 files changed

+20
-125
lines changed

4 files changed

+20
-125
lines changed

src/databricks/sql/auth/retry.py

Lines changed: 7 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ class CommandType(Enum):
3636
CLOSE_SESSION = "CloseSession"
3737
CLOSE_OPERATION = "CloseOperation"
3838
GET_OPERATION_STATUS = "GetOperationStatus"
39-
FEATURE_FLAGS = "FeatureFlags"
4039
OTHER = "Other"
4140

4241
@classmethod
@@ -374,6 +373,13 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
374373
if status_code == 403:
375374
return False, "403 codes are not retried"
376375

376+
# Request failed with 404. Don't retry for any command type.
377+
if status_code == 404:
378+
return (
379+
False,
380+
"Received 404 - NOT_FOUND. The requested resource does not exist.",
381+
)
382+
377383
# Request failed and server said NotImplemented. This isn't recoverable. Don't retry.
378384
if status_code == 501:
379385
return False, "Received code 501 from server."
@@ -382,41 +388,6 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
382388
if not self._is_method_retryable(method):
383389
return False, "Only POST requests are retried"
384390

385-
# Request failed with 404 and was a GetOperationStatus. This is not recoverable. Don't retry.
386-
if status_code == 404 and self.command_type == CommandType.GET_OPERATION_STATUS:
387-
return (
388-
False,
389-
"GetOperationStatus received 404 code from Databricks. Operation was canceled.",
390-
)
391-
392-
# Request failed with 404 because CloseSession returns 404 if you repeat the request.
393-
if (
394-
status_code == 404
395-
and self.command_type == CommandType.CLOSE_SESSION
396-
and len(self.history) > 0
397-
):
398-
raise SessionAlreadyClosedError(
399-
"CloseSession received 404 code from Databricks. Session is already closed."
400-
)
401-
402-
# Request failed with 404 because CloseOperation returns 404 if you repeat the request.
403-
if (
404-
status_code == 404
405-
and self.command_type == CommandType.CLOSE_OPERATION
406-
and len(self.history) > 0
407-
):
408-
raise CursorAlreadyClosedError(
409-
"CloseOperation received 404 code from Databricks. Cursor is already closed."
410-
)
411-
412-
# Request failed with 404 for feature flags endpoint (not available in Gov Cloud)
413-
# Feature flags are optional, so gracefully degrade without retrying
414-
if status_code == 404 and self.command_type == CommandType.FEATURE_FLAGS:
415-
return (
416-
False,
417-
"FeatureFlags endpoint returned 404. Feature flags are optional and will be disabled."
418-
)
419-
420391
# Request failed, was an ExecuteStatement and the command may have reached the server
421392
if (
422393
self.command_type == CommandType.EXECUTE_STATEMENT

src/databricks/sql/common/unified_http_client.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -251,15 +251,11 @@ def _prepare_headers(
251251

252252
return request_headers
253253

254-
def _prepare_retry_policy(self, url: str = ""):
254+
def _prepare_retry_policy(self):
255255
"""Set up the retry policy for the current request."""
256256
if isinstance(self._retry_policy, DatabricksRetryPolicy):
257-
# Detect feature flags endpoint by URL pattern
258-
if "/connector-service/feature-flags/" in url:
259-
self._retry_policy.command_type = CommandType.FEATURE_FLAGS
260-
else:
261-
# Set command type for HTTP requests to OTHER (not database commands)
262-
self._retry_policy.command_type = CommandType.OTHER
257+
# Set command type for HTTP requests to OTHER (not database commands)
258+
self._retry_policy.command_type = CommandType.OTHER
263259
# Start the retry timer for duration-based retry limits
264260
self._retry_policy.start_retry_timer()
265261

@@ -289,8 +285,8 @@ def request_context(
289285

290286
request_headers = self._prepare_headers(headers)
291287

292-
# Prepare retry policy for this request (pass url for detection)
293-
self._prepare_retry_policy(url)
288+
# Prepare retry policy for this request
289+
self._prepare_retry_policy()
294290

295291
# Select appropriate pool manager based on target URL
296292
pool_manager = self._get_pool_manager_for_url(url)

tests/unit/test_retry.py

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -84,22 +84,14 @@ def test_excessive_retry_attempts_error(self, t_mock, retry_policy):
8484
# Internally urllib3 calls the increment function generating a new instance for every retry
8585
retry_policy = retry_policy.increment()
8686

87-
def test_feature_flags_404_does_not_retry(self, retry_policy):
88-
"""Test that FEATURE_FLAGS CommandType with 404 should not retry"""
87+
def test_404_does_not_retry_for_any_command_type(self, retry_policy):
88+
"""Test that 404 never retries for any CommandType"""
8989
retry_policy._retry_start_time = time.time()
90-
retry_policy.command_type = CommandType.FEATURE_FLAGS
9190

92-
should_retry, msg = retry_policy.should_retry("POST", 404)
91+
# Test for each CommandType
92+
for command_type in CommandType:
93+
retry_policy.command_type = command_type
94+
should_retry, msg = retry_policy.should_retry("POST", 404)
9395

94-
assert should_retry is False
95-
assert "FeatureFlags endpoint returned 404" in msg
96-
assert "optional" in msg
97-
98-
def test_feature_flags_503_does_retry(self, retry_policy):
99-
"""Test that FEATURE_FLAGS CommandType with 503 should retry normally"""
100-
retry_policy._retry_start_time = time.time()
101-
retry_policy.command_type = CommandType.FEATURE_FLAGS
102-
103-
should_retry, msg = retry_policy.should_retry("POST", 503)
104-
105-
assert should_retry is True
96+
assert should_retry is False, f"404 should not retry for {command_type}"
97+
assert "404" in msg or "NOT_FOUND" in msg

tests/unit/test_unified_http_client.py

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
from databricks.sql.exc import RequestError
1313
from databricks.sql.auth.common import ClientContext
1414
from databricks.sql.types import SSLOptions
15-
from databricks.sql.auth.retry import CommandType
1615

1716

1817
class TestUnifiedHttpClientMaxRetryError:
@@ -135,66 +134,3 @@ def test_generic_exception_no_crash(self, http_client):
135134

136135
error = exc_info.value
137136
assert "HTTP request error" in str(error)
138-
139-
140-
class TestUnifiedHttpClientFeatureFlagsDetection:
141-
"""Test feature flags URL detection and CommandType setting."""
142-
143-
@pytest.fixture
144-
def client_context(self):
145-
"""Create a minimal ClientContext for testing."""
146-
context = Mock(spec=ClientContext)
147-
context.hostname = "https://test.databricks.com"
148-
context.ssl_options = SSLOptions(
149-
tls_verify=True,
150-
tls_verify_hostname=True,
151-
tls_trusted_ca_file=None,
152-
tls_client_cert_file=None,
153-
tls_client_cert_key_file=None,
154-
tls_client_cert_key_password=None,
155-
)
156-
context.socket_timeout = 30
157-
context.retry_stop_after_attempts_count = 3
158-
context.retry_delay_min = 1.0
159-
context.retry_delay_max = 10.0
160-
context.retry_stop_after_attempts_duration = 300.0
161-
context.retry_delay_default = 5.0
162-
context.retry_dangerous_codes = []
163-
context.proxy_auth_method = None
164-
context.pool_connections = 10
165-
context.pool_maxsize = 20
166-
context.user_agent = "test-agent"
167-
return context
168-
169-
@pytest.fixture
170-
def http_client(self, client_context):
171-
"""Create UnifiedHttpClient instance."""
172-
return UnifiedHttpClient(client_context)
173-
174-
def test_feature_flags_url_sets_feature_flags_command_type(self, http_client):
175-
"""Test that feature flags URL sets CommandType.FEATURE_FLAGS"""
176-
feature_flags_url = "https://test.databricks.com/connector-service/feature-flags/v1"
177-
178-
# Call _prepare_retry_policy with feature flags URL
179-
http_client._prepare_retry_policy(feature_flags_url)
180-
181-
# Verify CommandType is set to FEATURE_FLAGS
182-
assert http_client._retry_policy.command_type == CommandType.FEATURE_FLAGS
183-
184-
def test_non_feature_flags_url_sets_other_command_type(self, http_client):
185-
"""Test that non-feature-flags URLs set CommandType.OTHER"""
186-
normal_url = "https://test.databricks.com/api/2.0/sql/statements"
187-
188-
# Call _prepare_retry_policy with normal URL
189-
http_client._prepare_retry_policy(normal_url)
190-
191-
# Verify CommandType is set to OTHER
192-
assert http_client._retry_policy.command_type == CommandType.OTHER
193-
194-
def test_empty_url_sets_other_command_type(self, http_client):
195-
"""Test that empty URL defaults to CommandType.OTHER"""
196-
# Call _prepare_retry_policy with empty URL
197-
http_client._prepare_retry_policy("")
198-
199-
# Verify CommandType is set to OTHER
200-
assert http_client._retry_policy.command_type == CommandType.OTHER

0 commit comments

Comments
 (0)