Repository navigation
test: isolate retry HTTP mocks from background telemetry - #1055
Conversation
Signed-off-by: Vu Anh Phung <[email protected]>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good, with one low-severity concern. Scoping the urllib3 mocks to the test thread is a cleaner fix than tearing down the telemetry factory, and the new synchronized regression tests do reproduce the original contamination. The remaining gap is that mocked connections still get returned to the pool through the unpatched _put_conn.
Signed-off-by: Vu Anh Phung <[email protected]>
There was a problem hiding this comment.
Verdict: 1 Low
Looks good. Scoping the pool mocks to the test thread is a cleaner fix than tearing down the telemetry factory, and the synchronized unit tests cover both failure modes. One low concern: on urllib3 1.26's error path, a child MagicMock can still slip into the real pool. Nit: tests/unit now imports helpers from tests/e2e/common, which couples the unit suite to e2e modules; consider moving the shared helpers to a neutral test-utils module.
Signed-off-by: Vu Anh Phung <[email protected]>
What type of PR is this?
Description
Retry tests intermittently intercept background
/telemetry-extrequests, inflating HTTP call counts or consuming mocked response sequences. This has blocked three merge-queue attempts for #1027.Scope urllib3 pool acquisition and cleanup mocks to the calling test thread. Background calls use the original methods, fake connections stay out of the real queue, and the exact retry assertions remain. This replaces the telemetry-factory shutdown/restore workaround, which cannot drain work from retired executors.
How is this tested?
Passed 107 focused unit tests, 17 retry tests on both urllib3 1.26.20 and 2.7, and eight mocked retry E2E cases offline across Thrift and SEA. Synchronized regressions cover separate pools, shared pools, and transport-error cleanup. Full live warehouse validation runs in CI.
Related Tickets & Documents
#1027
https://github.com/databricks/databricks-sql-python/actions/runs/43524070623