Skip to content

Commit 471a551

Browse files
committed
Address review comments: revert timeout and telemetry_enabled changes
Per reviewer feedback on PR #734: 1. Revert timeout from 30s back to 900s (line 299) - Reviewer noted that with wait=False, timeout is not critical - The async nature and wait=False handle the exit speed 2. Revert telemetry_enabled parameter back to True (line 734) - Reviewer noted this is redundant given the early return - If enable_telemetry=False, we return early (line 729) - Line 734 only executes when enable_telemetry=True - Therefore using the parameter here is unnecessary These changes address the reviewer's valid technical concerns while keeping the core fixes intact: - wait=False for non-blocking shutdown (critical for Issue #729) - Early return when enable_telemetry=False (critical for Issue #729) - All Issue #731 fixes (null-safety, __del__, documentation) Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent bcdc8a5 commit 471a551

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

src/databricks/sql/telemetry/telemetry_client.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ def _send_telemetry(self, events):
296296
url,
297297
data=request.to_json(),
298298
headers=headers,
299-
timeout=30,
299+
timeout=900,
300300
)
301301

302302
future.add_done_callback(
@@ -731,7 +731,7 @@ def connection_failure_log(
731731
UNAUTH_DUMMY_SESSION_ID = "unauth_session_id"
732732

733733
TelemetryClientFactory.initialize_telemetry_client(
734-
telemetry_enabled=enable_telemetry,
734+
telemetry_enabled=True,
735735
session_id_hex=UNAUTH_DUMMY_SESSION_ID,
736736
auth_provider=None,
737737
host_url=host_url,

0 commit comments

Comments
 (0)