Fix #729 and #731: Telemetry lifecycle management

[msrathore-db]

What type of PR is this?

• Refactor
• Feature
• Bug Fix
• Other

Description

Issue #729: Connection failures hang CICD jobs for 15+ minutes, cannot be cancelled

• Telemetry forced on even with enable_telemetry=False
• 900-second timeout causes long hangs
• Blocking shutdown prevents process exit

Issue #731: Race condition causes AttributeError: 'NoneType' object has no attribute 'request'

• Async telemetry executes after _http_client is garbage collected
• No null check before calling pool_manager.request()

Changes

Issue #729 (3 fixes)

1. Respect enable_telemetry parameter (telemetry_client.py:722-729, client.py:321)
   - Added parameter to connection_failure_log() with early return if disabled
   - Connection passes user's preference through
2. Reduce timeout to 30s (telemetry_client.py:299)
   - Changed from 900s to 30s for faster failure
3. Non-blocking shutdown (telemetry_client.py:707)
   - Changed executor.shutdown(wait=True) to wait=False

Issue #731 (3 fixes)

4. Null-safety check (unified_http_client.py:296-298)
   - Check if pool_manager is None before calling .request()
   - Raise RequestError with clear message
5. Add del cleanup (telemetry_client.py:439-451)
   - Close _http_client during garbage collection
   - Ensures eventual resource cleanup
6. Document lifecycle (telemetry_client.py:430-435)
   - Added docstring explaining why _http_client isn't closed in close()
   - Import RequestError from databricks.sql.exc

Impact

• Connection failures exit in <1s instead of 15+ minutes
• CICD jobs no longer hang
• No more race condition AttributeError
• enable_telemetry=False now respected in all scenarios

Backward Compatibility

Fully backward compatible. No API changes, only behavior improvements.

Closes #729
Closes #731

How is this tested?

• Unit tests
• E2E Tests
• Manually
• N/A

Related Tickets & Documents

[github-actions]

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

[samikshya-db]

Thanks for the fix!

[samikshya-db]

There are retries etc that incorporate backoff, the push is anyway async, we should not be changing this in my opinion.

[samikshya-db]

nit : this change is not required.