Skip to content

Commit 41c4f40

Browse files
committed
Fix 60 seconds delay in gov cloud connections
1 parent 4edb046 commit 41c4f40

File tree

4 files changed

+102
-5
lines changed

4 files changed

+102
-5
lines changed

src/databricks/sql/auth/retry.py

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

4142
@classmethod
@@ -408,6 +409,14 @@ def should_retry(self, method: str, status_code: int) -> Tuple[bool, str]:
408409
"CloseOperation received 404 code from Databricks. Cursor is already closed."
409410
)
410411

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+
411420
# Request failed, was an ExecuteStatement and the command may have reached the server
412421
if (
413422
self.command_type == CommandType.EXECUTE_STATEMENT

src/databricks/sql/common/unified_http_client.py

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

252252
return request_headers
253253

254-
def _prepare_retry_policy(self):
254+
def _prepare_retry_policy(self, url: str = ""):
255255
"""Set up the retry policy for the current request."""
256256
if isinstance(self._retry_policy, DatabricksRetryPolicy):
257-
# Set command type for HTTP requests to OTHER (not database commands)
258-
self._retry_policy.command_type = CommandType.OTHER
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
259263
# Start the retry timer for duration-based retry limits
260264
self._retry_policy.start_retry_timer()
261265

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

286290
request_headers = self._prepare_headers(headers)
287291

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

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

tests/unit/test_retry.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,23 @@ def test_excessive_retry_attempts_error(self, t_mock, retry_policy):
8383
retry_policy.sleep(HTTPResponse(status=503))
8484
# Internally urllib3 calls the increment function generating a new instance for every retry
8585
retry_policy = retry_policy.increment()
86+
87+
def test_feature_flags_404_does_not_retry(self, retry_policy):
88+
"""Test that FEATURE_FLAGS CommandType with 404 should not retry"""
89+
retry_policy._retry_start_time = time.time()
90+
retry_policy.command_type = CommandType.FEATURE_FLAGS
91+
92+
should_retry, msg = retry_policy.should_retry("POST", 404)
93+
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

tests/unit/test_unified_http_client.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
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
1516

1617

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

135136
error = exc_info.value
136137
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)