Skip to content

Commit 60b38c5

Browse files
fix: cache BigQuery client and surface query errors (#115) (#116)
* fix: cache BigQuery client and surface query errors (#115) - get_client() now constructs the client once per resource instance and reuses it; previously every call re-resolved ADC credentials, and a single upsert created three clients - the location config field is now passed to the client (was silently ignored) - execute_query() no longer swallows all exceptions into an empty DataFrame; query failures raise so they surface as asset failures. DML/DDL statements (no result schema) still return an empty frame - fix self-referential class docstring left over from the MotherDuck migration - add unit tests for caching, location passthrough, error surfacing, and DML handling Closes #115 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style: apply ruff format --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 25ce43c commit 60b38c5

2 files changed

Lines changed: 115 additions & 8 deletions

File tree

macro_agents/src/macro_agents/defs/resources/bigquery_warehouse.py

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import polars as pl
1212
from google.cloud import bigquery
1313
from google.cloud.exceptions import NotFound
14-
from pydantic import Field
14+
from pydantic import Field, PrivateAttr
1515

1616
_IDENTIFIER_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*){0,2}$")
1717

@@ -24,14 +24,21 @@ def _validate_identifier(name: str, kind: str = "identifier") -> str:
2424

2525

2626
class BigQueryWarehouseResource(dg.ConfigurableResource):
27-
"""Dagster resource for BigQuery providing the same public API as BigQueryWarehouseResource."""
27+
"""Dagster resource for BigQuery.
28+
29+
Exposes the same public API as the retired MotherDuckResource so assets
30+
written against it work unchanged (see the get_connection and
31+
drop_create_duck_db_table aliases).
32+
"""
2833

2934
project: str = Field(description="GCP project ID")
3035
dataset: str = Field(
3136
description="Default BigQuery dataset", default="economics_raw"
3237
)
3338
location: str = Field(description="BigQuery location", default="US")
3439

40+
_client: bigquery.Client | None = PrivateAttr(default=None)
41+
3542
def _prepare_google_application_credentials(self) -> None:
3643
"""Support either a credentials file path or inline service-account JSON."""
3744
credentials = os.getenv("GOOGLE_APPLICATION_CREDENTIALS", "").strip()
@@ -53,9 +60,15 @@ def _prepare_google_application_credentials(self) -> None:
5360
os.environ["GOOGLE_APPLICATION_CREDENTIALS"] = credentials_path
5461

5562
def get_client(self) -> bigquery.Client:
56-
"""Return a BigQuery client. On GCE the VM service account is used via ADC."""
57-
self._prepare_google_application_credentials()
58-
return bigquery.Client(project=self.project)
63+
"""Return a cached BigQuery client. On GCE the VM service account is used via ADC.
64+
65+
Client construction resolves ADC credentials, so the client is
66+
created once per resource instance and reused across calls.
67+
"""
68+
if self._client is None:
69+
self._prepare_google_application_credentials()
70+
self._client = bigquery.Client(project=self.project, location=self.location)
71+
return self._client
5972

6073
def get_connection(self) -> bigquery.Client:
6174
"""Alias for get_client() — drop-in replacement for MotherDuckResource.get_connection()."""
@@ -167,6 +180,11 @@ def execute_query(
167180
168181
Bare table names are resolved against self.dataset so callers don't
169182
need to fully qualify every reference.
183+
184+
Query failures (bad SQL, missing tables, permissions) raise so they
185+
surface as asset failures instead of masquerading as empty result
186+
sets. DML/DDL statements, which produce no row schema, return an
187+
empty DataFrame.
170188
"""
171189
from google.cloud.bigquery import DatasetReference, QueryJobConfig
172190

@@ -175,10 +193,11 @@ def execute_query(
175193
default_dataset=DatasetReference(self.project, self.dataset)
176194
)
177195
job = client.query(query, job_config=job_config)
178-
try:
179-
return pl.DataFrame(job.result().to_arrow())
180-
except Exception:
196+
result = job.result()
197+
if not result.schema:
198+
# DML/DDL statements have no result rows to convert
181199
return pl.DataFrame()
200+
return pl.DataFrame(result.to_arrow())
182201

183202
def table_exists(self, table_name: str) -> bool:
184203
"""Check whether a table exists in BigQuery."""
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
"""Tests for BigQueryWarehouseResource client caching and error surfacing (issue #115)."""
2+
3+
from unittest.mock import Mock, patch
4+
5+
import polars as pl
6+
import pyarrow as pa
7+
import pytest
8+
9+
from macro_agents.defs.resources.bigquery_warehouse import BigQueryWarehouseResource
10+
11+
12+
def _make_resource() -> BigQueryWarehouseResource:
13+
return BigQueryWarehouseResource(project="test-project", dataset="test_dataset")
14+
15+
16+
class TestGetClientCaching:
17+
@patch("macro_agents.defs.resources.bigquery_warehouse.bigquery.Client")
18+
def test_client_constructed_once_across_calls(self, mock_client_cls):
19+
resource = _make_resource()
20+
21+
first = resource.get_client()
22+
second = resource.get_client()
23+
24+
assert first is second
25+
mock_client_cls.assert_called_once_with(project="test-project", location="US")
26+
27+
@patch("macro_agents.defs.resources.bigquery_warehouse.bigquery.Client")
28+
def test_location_config_is_passed_through(self, mock_client_cls):
29+
resource = BigQueryWarehouseResource(
30+
project="test-project", dataset="test_dataset", location="EU"
31+
)
32+
33+
resource.get_client()
34+
35+
mock_client_cls.assert_called_once_with(project="test-project", location="EU")
36+
37+
@patch("macro_agents.defs.resources.bigquery_warehouse.bigquery.Client")
38+
def test_get_connection_reuses_cached_client(self, mock_client_cls):
39+
resource = _make_resource()
40+
41+
client = resource.get_client()
42+
connection = resource.get_connection()
43+
44+
assert connection is client
45+
mock_client_cls.assert_called_once()
46+
47+
48+
class TestExecuteQueryErrorSurfacing:
49+
@patch("macro_agents.defs.resources.bigquery_warehouse.bigquery.Client")
50+
def test_query_failure_raises_instead_of_returning_empty(self, mock_client_cls):
51+
mock_job = Mock()
52+
mock_job.result.side_effect = RuntimeError("Table not found: missing_table")
53+
mock_client_cls.return_value.query.return_value = mock_job
54+
resource = _make_resource()
55+
56+
with pytest.raises(RuntimeError, match="missing_table"):
57+
resource.execute_query("SELECT * FROM missing_table")
58+
59+
@patch("macro_agents.defs.resources.bigquery_warehouse.bigquery.Client")
60+
def test_select_returns_polars_dataframe(self, mock_client_cls):
61+
arrow_table = pa.table({"value": [1, 2, 3]})
62+
mock_result = Mock()
63+
mock_result.schema = ["value"]
64+
mock_result.to_arrow.return_value = arrow_table
65+
mock_job = Mock()
66+
mock_job.result.return_value = mock_result
67+
mock_client_cls.return_value.query.return_value = mock_job
68+
resource = _make_resource()
69+
70+
df = resource.execute_query("SELECT value FROM some_table")
71+
72+
assert isinstance(df, pl.DataFrame)
73+
assert df["value"].to_list() == [1, 2, 3]
74+
75+
@patch("macro_agents.defs.resources.bigquery_warehouse.bigquery.Client")
76+
def test_dml_statement_returns_empty_dataframe(self, mock_client_cls):
77+
mock_result = Mock()
78+
mock_result.schema = []
79+
mock_job = Mock()
80+
mock_job.result.return_value = mock_result
81+
mock_client_cls.return_value.query.return_value = mock_job
82+
resource = _make_resource()
83+
84+
df = resource.execute_query("UPDATE some_table SET x = 1 WHERE TRUE")
85+
86+
assert isinstance(df, pl.DataFrame)
87+
assert df.is_empty()
88+
mock_result.to_arrow.assert_not_called()

0 commit comments

Comments
 (0)