Skip to content

Commit f2ec799

Browse files
committed
test(bigquery): refactor request interceptor to resolve socket leaks in system tests
1 parent 5f5f6c9 commit f2ec799

7 files changed

Lines changed: 188 additions & 59 deletions

File tree

packages/google-cloud-bigquery/google/cloud/bigquery/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@
123123
except ImportError:
124124
bigquery_magics = None
125125

126-
if sys.version_info < (3, 10):
126+
if sys.version_info < (3, 10): # pragma: NO COVER
127127
warnings.warn(
128128
"The python-bigquery library no longer supports Python <= 3.9. "
129129
f"Your Python version is {sys.version_info.major}.{sys.version_info.minor}.{sys.version_info.micro}. We "

packages/google-cloud-bigquery/google/cloud/bigquery/table.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2353,7 +2353,9 @@ def to_arrow(
23532353
progress_bar.close()
23542354
finally:
23552355
if owns_bqstorage_client:
2356-
bqstorage_client._transport.close()
2356+
# mypy: bqstorage_client is guaranteed to be not None when owns_bqstorage_client is True,
2357+
# but mypy cannot infer this correlation. We ignore the union-attr error here.
2358+
bqstorage_client._transport.close() # type: ignore[union-attr]
23572359

23582360
if record_batches and bqstorage_client is not None:
23592361
return pyarrow.Table.from_batches(record_batches)

packages/google-cloud-bigquery/tests/system/helpers.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import contextlib
1516
import datetime
1617
import decimal
1718
import uuid
@@ -104,3 +105,30 @@ def _rate_limit_exceeded(forbidden):
104105
google.api_core.exceptions.Forbidden,
105106
error_predicate=_rate_limit_exceeded,
106107
)
108+
109+
110+
@contextlib.contextmanager
111+
def patch_tracked_requests():
112+
"""Context manager to patch google-auth requests and track/close their HTTP sessions.
113+
114+
This prevents socket leaks in system tests that use Workload Identity or metadata server auth.
115+
"""
116+
import contextlib
117+
import google.auth.transport.requests
118+
119+
original_init = google.auth.transport.requests.Request.__init__
120+
tracked_requests = []
121+
122+
def patched_init(self, session=None):
123+
original_init(self, session=session)
124+
tracked_requests.append(self)
125+
126+
google.auth.transport.requests.Request.__init__ = patched_init
127+
try:
128+
yield tracked_requests
129+
finally:
130+
google.auth.transport.requests.Request.__init__ = original_init
131+
for req in tracked_requests:
132+
if hasattr(req, "session") and req.session is not None:
133+
req.session.close()
134+

packages/google-cloud-bigquery/tests/system/test_client.py

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
# limitations under the License.
1414

1515
import base64
16+
import contextlib
17+
import google.auth.transport.requests
1618
import copy
1719
import csv
1820
import datetime
@@ -155,6 +157,8 @@ def _load_json_schema(filename="schema.json"):
155157
return _parse_schema_resource(json.load(schema_file))
156158

157159

160+
161+
158162
class Config(object):
159163
"""Run-time configuration to be modified at set-up.
160164
@@ -234,23 +238,29 @@ def _create_bucket(self, bucket_name, location=None):
234238

235239
def test_close_releases_open_sockets(self):
236240
current_process = psutil.Process()
237-
conn_count_start = len(current_process.net_connections())
241+
conn_start = current_process.net_connections()
242+
conn_count_start = len(conn_start)
243+
244+
with helpers.patch_tracked_requests():
245+
client = Config.CLIENT
246+
client.query(
247+
"""
248+
SELECT
249+
source_year AS year, COUNT(is_male) AS birth_count
250+
FROM `bigquery-public-data.samples.natality`
251+
GROUP BY year
252+
ORDER BY year DESC
253+
LIMIT 15
254+
"""
255+
)
238256

239-
client = Config.CLIENT
240-
client.query(
241-
"""
242-
SELECT
243-
source_year AS year, COUNT(is_male) AS birth_count
244-
FROM `bigquery-public-data.samples.natality`
245-
GROUP BY year
246-
ORDER BY year DESC
247-
LIMIT 15
248-
"""
249-
)
257+
client.close()
250258

251-
client.close()
259+
import gc
252260

253-
conn_count_end = len(current_process.net_connections())
261+
gc.collect()
262+
conn_end = current_process.net_connections()
263+
conn_count_end = len(conn_end)
254264
self.assertLessEqual(conn_count_end, conn_count_start)
255265

256266
def test_create_dataset(self):
@@ -2174,25 +2184,31 @@ def test_dbapi_dry_run_query(self):
21742184
def test_dbapi_connection_does_not_leak_sockets(self):
21752185
pytest.importorskip("google.cloud.bigquery_storage")
21762186
current_process = psutil.Process()
2177-
conn_count_start = len(current_process.net_connections())
2178-
2179-
# Provide no explicit clients, so that the connection will create and own them.
2180-
connection = dbapi.connect()
2181-
cursor = connection.cursor()
2182-
2183-
cursor.execute(
2187+
conn_start = current_process.net_connections()
2188+
conn_count_start = len(conn_start)
2189+
2190+
with helpers.patch_tracked_requests():
2191+
# Provide no explicit clients, so that the connection will create and own them.
2192+
connection = dbapi.connect()
2193+
cursor = connection.cursor()
2194+
2195+
cursor.execute(
2196+
"""
2197+
SELECT id, `by`, timestamp
2198+
FROM `bigquery-public-data.hacker_news.full`
2199+
ORDER BY `id` ASC
2200+
LIMIT 100000
21842201
"""
2185-
SELECT id, `by`, timestamp
2186-
FROM `bigquery-public-data.hacker_news.full`
2187-
ORDER BY `id` ASC
2188-
LIMIT 100000
2189-
"""
2190-
)
2191-
rows = cursor.fetchall()
2192-
self.assertEqual(len(rows), 100000)
2202+
)
2203+
rows = cursor.fetchall()
2204+
self.assertEqual(len(rows), 100000)
2205+
2206+
connection.close()
2207+
import gc
21932208

2194-
connection.close()
2195-
conn_count_end = len(current_process.net_connections())
2209+
gc.collect()
2210+
conn_end = current_process.net_connections()
2211+
conn_count_end = len(conn_end)
21962212
self.assertLessEqual(conn_count_end, conn_count_start)
21972213

21982214
def _load_table_for_dml(self, rows, dataset_id, table_id):

packages/google-cloud-bigquery/tests/system/test_magics.py

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,15 @@
1414

1515
"""System tests for Jupyter/IPython connector."""
1616

17+
import contextlib
1718
import re
1819

20+
import google.auth.transport.requests
21+
1922
import pytest
2023
import psutil
2124

25+
from . import helpers
2226

2327
IPython = pytest.importorskip("IPython")
2428
io = pytest.importorskip("IPython.utils.io")
@@ -45,30 +49,35 @@ def ipython_interactive(ipython):
4549
yield ipython
4650

4751

52+
53+
4854
def test_bigquery_magic(ipython_interactive):
4955
ip = IPython.get_ipython()
5056
current_process = psutil.Process()
51-
conn_count_start = len(current_process.net_connections())
52-
53-
# Deprecated, but should still work in google-cloud-bigquery 3.x.
54-
with pytest.warns(FutureWarning, match="bigquery_magics"):
55-
ip.extension_manager.load_extension("google.cloud.bigquery")
56-
57-
sql = """
58-
SELECT
59-
CONCAT(
60-
'https://stackoverflow.com/questions/',
61-
CAST(id as STRING)) as url,
62-
view_count
63-
FROM `bigquery-public-data.stackoverflow.posts_questions`
64-
WHERE tags like '%google-bigquery%'
65-
ORDER BY view_count DESC
66-
LIMIT 10
67-
"""
68-
with io.capture_output() as captured:
69-
result = ip.run_cell_magic("bigquery", "--use_rest_api", sql)
70-
71-
conn_count_end = len(current_process.net_connections())
57+
conn_start = current_process.net_connections()
58+
conn_count_start = len(conn_start)
59+
60+
with helpers.patch_tracked_requests():
61+
# Deprecated, but should still work in google-cloud-bigquery 3.x.
62+
with pytest.warns(FutureWarning, match="bigquery_magics"):
63+
ip.extension_manager.load_extension("google.cloud.bigquery")
64+
65+
sql = """
66+
SELECT
67+
CONCAT(
68+
'https://stackoverflow.com/questions/',
69+
CAST(id as STRING)) as url,
70+
view_count
71+
FROM `bigquery-public-data.stackoverflow.posts_questions`
72+
WHERE tags like '%google-bigquery%'
73+
ORDER BY view_count DESC
74+
LIMIT 10
75+
"""
76+
with io.capture_output() as captured:
77+
result = ip.run_cell_magic("bigquery", "--use_rest_api", sql)
78+
79+
conn_end = current_process.net_connections()
80+
conn_count_end = len(conn_end)
7281

7382
lines = re.split("\n|\r", captured.stdout)
7483
# Removes blanks & terminal code (result of display clearing)
@@ -83,3 +92,4 @@ def test_bigquery_magic(ipython_interactive):
8392
# than expected when running system tests on Kokoro, thus using the <= assertion.
8493
# That's still fine, however, since the sockets are apparently not leaked.
8594
assert conn_count_end <= conn_count_start # system resources are released
95+

packages/google-cloud-bigquery/tests/unit/test_magics.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545

4646
@pytest.fixture()
4747
def use_local_magics_context(monkeypatch):
48-
if magics is not None:
48+
if magics is not None: # pragma: NO COVER
4949
local_context = magics.Context()
5050
local_context._project = "unit-test-project"
5151
mock_credentials = mock.create_autospec(
@@ -2195,13 +2195,10 @@ def test_bigquery_magic_create_dataset_fails(monkeypatch):
21952195

21962196

21972197
@pytest.mark.usefixtures("ipython_interactive")
2198-
def test_bigquery_magic_with_location(monkeypatch):
2198+
def test_bigquery_magic_with_location(monkeypatch, use_local_magics_context):
21992199
ip = IPython.get_ipython()
22002200
monkeypatch.setattr(bigquery, "bigquery_magics", None)
22012201
bigquery.load_ipython_extension(ip)
2202-
magics.context.credentials = mock.create_autospec(
2203-
google.auth.credentials.Credentials, instance=True
2204-
)
22052202

22062203
run_query_patch = mock.patch(
22072204
"google.cloud.bigquery.magics.magics._run_query", autospec=True

packages/google-cloud-bigquery/tests/unit/test_table.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,82 @@ def test_ctor_with_key(self):
7979
self.assertEqual(encryption_config.kms_key_name, self.KMS_KEY_NAME)
8080

8181

82+
class TestPropertyGraphReference(unittest.TestCase):
83+
PROJECT = "my-project"
84+
DATASET_ID = "my_dataset"
85+
PROPERTY_GRAPH_ID = "my_pg"
86+
87+
def _get_target_class(self):
88+
from google.cloud.bigquery.table import PropertyGraphReference
89+
90+
return PropertyGraphReference
91+
92+
def _make_one(self, *args, **kw):
93+
return self._get_target_class()(*args, **kw)
94+
95+
def test_ctor(self):
96+
dataset_ref = DatasetReference(self.PROJECT, self.DATASET_ID)
97+
ref = self._make_one(dataset_ref, self.PROPERTY_GRAPH_ID)
98+
self.assertEqual(ref.project, self.PROJECT)
99+
self.assertEqual(ref.dataset_id, self.DATASET_ID)
100+
self.assertEqual(ref.property_graph_id, self.PROPERTY_GRAPH_ID)
101+
102+
def test_from_api_repr(self):
103+
resource = {
104+
"projectId": self.PROJECT,
105+
"datasetId": self.DATASET_ID,
106+
"propertyGraphId": self.PROPERTY_GRAPH_ID,
107+
}
108+
ref = self._get_target_class().from_api_repr(resource)
109+
self.assertEqual(ref.project, self.PROJECT)
110+
self.assertEqual(ref.dataset_id, self.DATASET_ID)
111+
self.assertEqual(ref.property_graph_id, self.PROPERTY_GRAPH_ID)
112+
113+
def test_to_api_repr(self):
114+
dataset_ref = DatasetReference(self.PROJECT, self.DATASET_ID)
115+
ref = self._make_one(dataset_ref, self.PROPERTY_GRAPH_ID)
116+
resource = ref.to_api_repr()
117+
expected = {
118+
"projectId": self.PROJECT,
119+
"datasetId": self.DATASET_ID,
120+
"propertyGraphId": self.PROPERTY_GRAPH_ID,
121+
}
122+
self.assertEqual(resource, expected)
123+
124+
def test___str__(self):
125+
dataset_ref = DatasetReference(self.PROJECT, self.DATASET_ID)
126+
ref = self._make_one(dataset_ref, self.PROPERTY_GRAPH_ID)
127+
self.assertEqual(
128+
str(ref), f"{self.PROJECT}.{self.DATASET_ID}.{self.PROPERTY_GRAPH_ID}"
129+
)
130+
131+
def test___repr__(self):
132+
dataset_ref = DatasetReference(self.PROJECT, self.DATASET_ID)
133+
ref = self._make_one(dataset_ref, self.PROPERTY_GRAPH_ID)
134+
expected = (
135+
f"PropertyGraphReference({dataset_ref!r}, '{self.PROPERTY_GRAPH_ID}')"
136+
)
137+
self.assertEqual(repr(ref), expected)
138+
139+
def test___eq__(self):
140+
dataset_ref1 = DatasetReference(self.PROJECT, self.DATASET_ID)
141+
ref1 = self._make_one(dataset_ref1, self.PROPERTY_GRAPH_ID)
142+
dataset_ref2 = DatasetReference(self.PROJECT, self.DATASET_ID)
143+
ref2 = self._make_one(dataset_ref2, self.PROPERTY_GRAPH_ID)
144+
self.assertEqual(ref1, ref2)
145+
146+
ref3 = self._make_one(dataset_ref1, "other_pg")
147+
self.assertNotEqual(ref1, ref3)
148+
self.assertNotEqual(ref1, object())
149+
150+
def test___hash__(self):
151+
dataset_ref1 = DatasetReference(self.PROJECT, self.DATASET_ID)
152+
ref1 = self._make_one(dataset_ref1, self.PROPERTY_GRAPH_ID)
153+
dataset_ref2 = DatasetReference(self.PROJECT, self.DATASET_ID)
154+
ref2 = self._make_one(dataset_ref2, self.PROPERTY_GRAPH_ID)
155+
self.assertEqual(hash(ref1), hash(ref2))
156+
157+
82158
class TestTableBase:
83159
@staticmethod
84160
def _get_target_class():

0 commit comments

Comments
 (0)