fix(bigquery): harden system test teardown and unit test auth isolation - #17956
Closed
shuoweil wants to merge 3 commits into
Closed
fix(bigquery): harden system test teardown and unit test auth isolation#17956shuoweil wants to merge 3 commits into
shuoweil wants to merge 3 commits into
Conversation
shuoweil
requested review from
GarrettWu,
sycai and
tswast
and removed request for
a team
July 30, 2026 21:03
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request improves resource cleanup and socket leak prevention in the BigQuery DB-API connection implementation, alongside updating test assertions and IPython magic tests. Specifically, it ensures cursors are closed prior to closing the client connections, handles NotFound exceptions during tag deletions in system tests, and updates the socket leak test to track specific connection addresses and explicitly close cursors. Additionally, unit tests for IPython magics are updated to use IPython.core.interactiveshell and modern parenthesized context managers. Review feedback suggests wrapping the cursor closing loop in a try...except block to prevent an exception in one cursor from blocking the closure of other cursors and the underlying clients.
…nection.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Contributor
|
Googlers see b/540939659 |
Contributor
Author
Thanks @parthea. I will trim down this PR and update the bug. |
shuoweil
pushed a commit
that referenced
this pull request
Jul 31, 2026
This Pull Request addresses flakiness in the socket leak system test and improves resource cleanup in the Database API (DB-API) connection. ## Problem 1. The test `test_dbapi_connection_does_not_leak_sockets` was flaky in Continuous Integration (CI) environments because it relied on the `psutil` library to count Operating System sockets. Socket pooling, delayed garbage collection, and background noise (other processes in multiprocess systems creating sockets in parallelized systems) make counting Operating System sockets non-deterministic. 2. Closing a database connection could leave underlying resources hanging if cursors were not closed in the correct order or if clients were not fully initialized. ## Solution 1. **Deterministic Testing:** Rewrote the test to use **Whitebox Object Tracking**. Instead of counting Operating System sockets, the test intercepts the creation and closure of `requests.Session` and `grpc.Channel` objects using instance-level monkey patching. This ensures that every session or channel opened by the connection is explicitly accounted for and closed, independent of Operating System quirks. 2. **Improved Cleanup:** Updated `Connection.close()` in `connection.py` to close all created cursors **before** closing the clients themselves. This ensures that cursors release their references to query data and transports first. Added defensive checks to prevent errors if clients are `None`. > [!note] > - This test no longer uses `psutil` or artificial sleeps (that were used to try and wait long enough for garbage collection to kick in), making it faster and fully deterministic. > - Uses Python `sets` to track objects thus avoiding false positives due to double closures. > - Instance-level patching is used to isolate tracking to only the objects created within the test block. Closes #17956
shuoweil
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following the merge of PR #17953 for the socket leak fix, this PR focuses on the remaining orthogonal test and fixture hardening fixes:
try...except NotFound: passduringTestBigQuery.tearDown()to prevent cascaded teardown failures when tag resources were already deleted.autouse=Trueon theuse_local_magics_contextfixture intest_magics.pyto prevent credentials mutation across test runs in uncredentialed CI environments.IPython.core.interactiveshell.InteractiveShelland add fallback unit test coverage formagics.Context.