Skip to content

Commit 8cdfd88

Browse files
committed
Perf update
1 parent 0cbfae6 commit 8cdfd88

File tree

3 files changed

+60
-52
lines changed

3 files changed

+60
-52
lines changed

src/databricks/sql/client.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1514,7 +1514,7 @@ def fetchall_arrow(self) -> "pyarrow.Table":
15141514
"""Fetch all (remaining) rows of a query result, returning them as a PyArrow table."""
15151515
results = self.results.remaining_rows()
15161516
self._next_row_index += results.num_rows
1517-
1517+
15181518
partial_result_chunks = [results]
15191519
while not self.has_been_closed_server_side and self.has_more_rows:
15201520
self._fill_results_buffer()

src/databricks/sql/cloudfetch/downloader.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ def run(self) -> DownloadedFile:
8989
ResultSetDownloadHandler._validate_link(
9090
self.link, self.settings.link_expiry_buffer_secs
9191
)
92-
92+
9393
with self._http_client.execute(
9494
method=HttpMethod.GET,
9595
url=self.link.fileLink,

tests/unit/test_telemetry.py

Lines changed: 58 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
NoopTelemetryClient,
99
TelemetryClientFactory,
1010
TelemetryHelper,
11-
BaseTelemetryClient
11+
BaseTelemetryClient,
1212
)
1313
from databricks.sql.telemetry.models.enums import AuthMech, AuthFlow
1414
from databricks.sql.auth.authenticators import (
@@ -24,7 +24,7 @@ def mock_telemetry_client():
2424
session_id = str(uuid.uuid4())
2525
auth_provider = AccessTokenAuthProvider("test-token")
2626
executor = MagicMock()
27-
27+
2828
return TelemetryClient(
2929
telemetry_enabled=True,
3030
session_id_hex=session_id,
@@ -43,7 +43,7 @@ def test_noop_client_behavior(self):
4343
client1 = NoopTelemetryClient()
4444
client2 = NoopTelemetryClient()
4545
assert client1 is client2
46-
46+
4747
# Test that all methods can be called without exceptions
4848
client1.export_initial_telemetry_log(MagicMock(), "test-agent")
4949
client1.export_failure_log("TestError", "Test message")
@@ -58,76 +58,76 @@ def test_event_batching_and_flushing_flow(self, mock_telemetry_client):
5858
"""Test the complete event batching and flushing flow."""
5959
client = mock_telemetry_client
6060
client._batch_size = 3 # Small batch for testing
61-
61+
6262
# Mock the network call
63-
with patch.object(client, '_send_telemetry') as mock_send:
63+
with patch.object(client, "_send_telemetry") as mock_send:
6464
# Add events one by one - should not flush yet
6565
client._export_event("event1")
6666
client._export_event("event2")
6767
mock_send.assert_not_called()
6868
assert len(client._events_batch) == 2
69-
69+
7070
# Third event should trigger flush
7171
client._export_event("event3")
7272
mock_send.assert_called_once()
7373
assert len(client._events_batch) == 0 # Batch cleared after flush
74-
75-
@patch('requests.post')
74+
75+
@patch("requests.post")
7676
def test_network_request_flow(self, mock_post, mock_telemetry_client):
7777
"""Test the complete network request flow with authentication."""
7878
mock_post.return_value.status_code = 200
7979
client = mock_telemetry_client
80-
80+
8181
# Create mock events
8282
mock_events = [MagicMock() for _ in range(2)]
8383
for i, event in enumerate(mock_events):
8484
event.to_json.return_value = f'{{"event": "{i}"}}'
85-
85+
8686
# Send telemetry
8787
client._send_telemetry(mock_events)
88-
88+
8989
# Verify request was submitted to executor
9090
client._executor.submit.assert_called_once()
9191
args, kwargs = client._executor.submit.call_args
92-
92+
9393
# Verify correct function and URL
9494
assert args[0] == requests.post
95-
assert args[1] == 'https://test-host.com/telemetry-ext'
96-
assert kwargs['headers']['Authorization'] == 'Bearer test-token'
97-
95+
assert args[1] == "https://test-host.com/telemetry-ext"
96+
assert kwargs["headers"]["Authorization"] == "Bearer test-token"
97+
9898
# Verify request body structure
99-
request_data = kwargs['data']
99+
request_data = kwargs["data"]
100100
assert '"uploadTime"' in request_data
101101
assert '"protoLogs"' in request_data
102102

103103
def test_telemetry_logging_flows(self, mock_telemetry_client):
104104
"""Test all telemetry logging methods work end-to-end."""
105105
client = mock_telemetry_client
106-
107-
with patch.object(client, '_export_event') as mock_export:
106+
107+
with patch.object(client, "_export_event") as mock_export:
108108
# Test initial log
109109
client.export_initial_telemetry_log(MagicMock(), "test-agent")
110110
assert mock_export.call_count == 1
111-
111+
112112
# Test failure log
113113
client.export_failure_log("TestError", "Error message")
114114
assert mock_export.call_count == 2
115-
115+
116116
# Test latency log
117117
client.export_latency_log(150, "EXECUTE_STATEMENT", "stmt-123")
118118
assert mock_export.call_count == 3
119119

120120
def test_error_handling_resilience(self, mock_telemetry_client):
121121
"""Test that telemetry errors don't break the client."""
122122
client = mock_telemetry_client
123-
123+
124124
# Test that exceptions in telemetry don't propagate
125-
with patch.object(client, '_export_event', side_effect=Exception("Test error")):
125+
with patch.object(client, "_export_event", side_effect=Exception("Test error")):
126126
# These should not raise exceptions
127127
client.export_initial_telemetry_log(MagicMock(), "test-agent")
128128
client.export_failure_log("TestError", "Error message")
129129
client.export_latency_log(100, "EXECUTE_STATEMENT", "stmt-123")
130-
130+
131131
# Test executor submission failure
132132
client._executor.submit.side_effect = Exception("Thread pool error")
133133
client._send_telemetry([MagicMock()]) # Should not raise
@@ -140,7 +140,7 @@ def test_system_configuration_caching(self):
140140
"""Test that system configuration is cached and contains expected data."""
141141
config1 = TelemetryHelper.get_driver_system_configuration()
142142
config2 = TelemetryHelper.get_driver_system_configuration()
143-
143+
144144
# Should be cached (same instance)
145145
assert config1 is config2
146146

@@ -153,7 +153,7 @@ def test_auth_mechanism_detection(self):
153153
(MagicMock(), AuthMech.OTHER), # Unknown provider
154154
(None, None),
155155
]
156-
156+
157157
for provider, expected in test_cases:
158158
assert TelemetryHelper.get_auth_mechanism(provider) == expected
159159

@@ -163,19 +163,25 @@ def test_auth_flow_detection(self):
163163
oauth_with_tokens = MagicMock(spec=DatabricksOAuthProvider)
164164
oauth_with_tokens._access_token = "test-access-token"
165165
oauth_with_tokens._refresh_token = "test-refresh-token"
166-
assert TelemetryHelper.get_auth_flow(oauth_with_tokens) == AuthFlow.TOKEN_PASSTHROUGH
167-
166+
assert (
167+
TelemetryHelper.get_auth_flow(oauth_with_tokens)
168+
== AuthFlow.TOKEN_PASSTHROUGH
169+
)
170+
168171
# Test OAuth with browser-based auth
169172
oauth_with_browser = MagicMock(spec=DatabricksOAuthProvider)
170173
oauth_with_browser._access_token = None
171174
oauth_with_browser._refresh_token = None
172175
oauth_with_browser.oauth_manager = MagicMock()
173-
assert TelemetryHelper.get_auth_flow(oauth_with_browser) == AuthFlow.BROWSER_BASED_AUTHENTICATION
174-
176+
assert (
177+
TelemetryHelper.get_auth_flow(oauth_with_browser)
178+
== AuthFlow.BROWSER_BASED_AUTHENTICATION
179+
)
180+
175181
# Test non-OAuth provider
176182
pat_auth = AccessTokenAuthProvider("test-token")
177183
assert TelemetryHelper.get_auth_flow(pat_auth) is None
178-
184+
179185
# Test None auth provider
180186
assert TelemetryHelper.get_auth_flow(None) is None
181187

@@ -202,56 +208,58 @@ def test_client_lifecycle_flow(self):
202208
"""Test complete client lifecycle: initialize -> use -> close."""
203209
session_id_hex = "test-session"
204210
auth_provider = AccessTokenAuthProvider("token")
205-
211+
206212
# Initialize enabled client
207213
TelemetryClientFactory.initialize_telemetry_client(
208214
telemetry_enabled=True,
209215
session_id_hex=session_id_hex,
210216
auth_provider=auth_provider,
211-
host_url="test-host.com"
217+
host_url="test-host.com",
212218
)
213-
219+
214220
client = TelemetryClientFactory.get_telemetry_client(session_id_hex)
215221
assert isinstance(client, TelemetryClient)
216222
assert client._session_id_hex == session_id_hex
217-
223+
218224
# Close client
219-
with patch.object(client, 'close') as mock_close:
225+
with patch.object(client, "close") as mock_close:
220226
TelemetryClientFactory.close(session_id_hex)
221227
mock_close.assert_called_once()
222-
228+
223229
# Should get NoopTelemetryClient after close
224230
client = TelemetryClientFactory.get_telemetry_client(session_id_hex)
225231
assert isinstance(client, NoopTelemetryClient)
226232

227233
def test_disabled_telemetry_flow(self):
228234
"""Test that disabled telemetry uses NoopTelemetryClient."""
229235
session_id_hex = "test-session"
230-
236+
231237
TelemetryClientFactory.initialize_telemetry_client(
232238
telemetry_enabled=False,
233239
session_id_hex=session_id_hex,
234240
auth_provider=None,
235-
host_url="test-host.com"
241+
host_url="test-host.com",
236242
)
237-
243+
238244
client = TelemetryClientFactory.get_telemetry_client(session_id_hex)
239245
assert isinstance(client, NoopTelemetryClient)
240246

241247
def test_factory_error_handling(self):
242248
"""Test that factory errors fall back to NoopTelemetryClient."""
243249
session_id = "test-session"
244-
250+
245251
# Simulate initialization error
246-
with patch('databricks.sql.telemetry.telemetry_client.TelemetryClient',
247-
side_effect=Exception("Init error")):
252+
with patch(
253+
"databricks.sql.telemetry.telemetry_client.TelemetryClient",
254+
side_effect=Exception("Init error"),
255+
):
248256
TelemetryClientFactory.initialize_telemetry_client(
249257
telemetry_enabled=True,
250258
session_id_hex=session_id,
251259
auth_provider=AccessTokenAuthProvider("token"),
252-
host_url="test-host.com"
260+
host_url="test-host.com",
253261
)
254-
262+
255263
# Should fall back to NoopTelemetryClient
256264
client = TelemetryClientFactory.get_telemetry_client(session_id)
257265
assert isinstance(client, NoopTelemetryClient)
@@ -260,25 +268,25 @@ def test_factory_shutdown_flow(self):
260268
"""Test factory shutdown when last client is removed."""
261269
session1 = "session-1"
262270
session2 = "session-2"
263-
271+
264272
# Initialize multiple clients
265273
for session in [session1, session2]:
266274
TelemetryClientFactory.initialize_telemetry_client(
267275
telemetry_enabled=True,
268276
session_id_hex=session,
269277
auth_provider=AccessTokenAuthProvider("token"),
270-
host_url="test-host.com"
278+
host_url="test-host.com",
271279
)
272-
280+
273281
# Factory should be initialized
274282
assert TelemetryClientFactory._initialized is True
275283
assert TelemetryClientFactory._executor is not None
276-
284+
277285
# Close first client - factory should stay initialized
278286
TelemetryClientFactory.close(session1)
279287
assert TelemetryClientFactory._initialized is True
280-
288+
281289
# Close second client - factory should shut down
282290
TelemetryClientFactory.close(session2)
283291
assert TelemetryClientFactory._initialized is False
284-
assert TelemetryClientFactory._executor is None
292+
assert TelemetryClientFactory._executor is None

0 commit comments

Comments
 (0)