Skip to content

Commit c6f3960

Browse files
caohy1988claude
andcommitted
simplify: remove _resolved_location, let BQ infer location server-side
Per review feedback: when bigquery.Client is created without a location parameter, client.query() sends no location field in the API request. BigQuery infers the job location from the dataset referenced in the DDL statement. This is simpler than auto-detecting via get_dataset(). The only production change from origin/main is removing location=self.location from the bigquery.Client() constructor call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a50dc86 commit c6f3960

2 files changed

Lines changed: 20 additions & 72 deletions

File tree

src/google/adk/plugins/bigquery_agent_analytics_plugin.py

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1990,7 +1990,6 @@ def __init__(
19901990
self._setup_lock = None
19911991
self._user_credentials = credentials
19921992
self._credentials = credentials
1993-
self._resolved_location: Optional[str] = None
19941993
self.client = None
19951994
self._loop_state_by_loop: dict[asyncio.AbstractEventLoop, _LoopState] = {}
19961995
self._write_stream_name = None # Resolved stream name
@@ -2189,24 +2188,6 @@ async def _lazy_setup(self, **kwargs) -> None:
21892188
),
21902189
)
21912190

2192-
# Auto-detect the dataset's location so view-creation DDL
2193-
# queries are routed to the correct region. Table CRUD and the
2194-
# Storage Write API derive location from the dataset server-side,
2195-
# but client.query() uses the client's default location for job
2196-
# routing — a mismatch causes silent view-creation failure.
2197-
if self._resolved_location is None:
2198-
dataset_id = f"{self.project_id}.{self.dataset_id}"
2199-
try:
2200-
dataset = await loop.run_in_executor(
2201-
self._executor,
2202-
lambda: self.client.get_dataset(dataset_id),
2203-
)
2204-
self._resolved_location = dataset.location
2205-
except Exception:
2206-
# Fallback when dataset metadata cannot be resolved,
2207-
# preserving current behavior.
2208-
self._resolved_location = self.location
2209-
22102191
self.full_table_id = f"{self.project_id}.{self.dataset_id}.{self.table_id}"
22112192
if not self._schema:
22122193
self._schema = _get_events_schema()
@@ -2465,7 +2446,7 @@ def _create_analytics_views(self) -> None:
24652446
event_type=event_type,
24662447
)
24672448
try:
2468-
self.client.query(sql, location=self._resolved_location).result()
2449+
self.client.query(sql).result()
24692450
except cloud_exceptions.Conflict:
24702451
logger.debug(
24712452
"View %s was updated concurrently by another process.",
@@ -2564,8 +2545,6 @@ def __getstate__(self):
25642545
state["_startup_error"] = None
25652546
state["_is_shutting_down"] = False
25662547
state["_init_pid"] = 0
2567-
# Re-detect dataset location after unpickle.
2568-
state["_resolved_location"] = None
25692548
# _credentials is always runtime-resolved; clear unconditionally.
25702549
state["_credentials"] = None
25712550
# Preserve _user_credentials if they are picklable (e.g.,
@@ -2587,7 +2566,6 @@ def __setstate__(self, state):
25872566
state.setdefault("_init_pid", 0)
25882567
state.setdefault("_user_credentials", None)
25892568
state.setdefault("_credentials", None)
2590-
state.setdefault("_resolved_location", None)
25912569
# Restore _credentials from _user_credentials if available so
25922570
# _create_loop_state uses the user's identity. When both are
25932571
# None (non-picklable credentials were dropped), ADC is used.
@@ -2637,7 +2615,6 @@ def _reset_runtime_state(self) -> None:
26372615

26382616
# Clear all runtime state.
26392617
self._setup_lock = None
2640-
self._resolved_location = None
26412618
self.client = None
26422619
self._loop_state_by_loop = {}
26432620
self._write_stream_name = None

tests/unittests/plugins/test_bigquery_agent_analytics_plugin.py

Lines changed: 19 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -7249,50 +7249,26 @@ async def test_no_a2a_interaction_for_no_metadata(
72497249

72507250

72517251
# ================================================================
7252-
# TEST CLASS: Dataset location auto-detection (Issue #5476)
7252+
# TEST CLASS: Dataset location handling (Issue #5476)
72537253
# ================================================================
7254-
class TestDatasetLocationDetection:
7255-
"""Tests for auto-detecting the dataset location."""
7254+
class TestDatasetLocationHandling:
7255+
"""Tests that BQ client is created without a default location.
72567256
7257-
@pytest.mark.asyncio
7258-
async def test_location_detected_from_dataset(
7259-
self,
7260-
mock_auth_default,
7261-
mock_to_arrow_schema,
7262-
mock_asyncio_to_thread,
7263-
):
7264-
"""Resolved location comes from get_dataset, not constructor."""
7265-
mock_dataset = mock.MagicMock()
7266-
mock_dataset.location = "europe-west1"
7267-
7268-
with mock.patch.object(bigquery, "Client", autospec=True) as mock_bq_cls:
7269-
mock_bq_cls.return_value.get_dataset.return_value = mock_dataset
7270-
mock_bq_cls.return_value.get_table.side_effect = (
7271-
cloud_exceptions.NotFound("table")
7272-
)
7273-
mock_bq_cls.return_value.create_table.return_value = None
7274-
7275-
async with managed_plugin(
7276-
project_id=PROJECT_ID,
7277-
dataset_id=DATASET_ID,
7278-
table_id=TABLE_ID,
7279-
config=bigquery_agent_analytics_plugin.BigQueryLoggerConfig(
7280-
create_views=False,
7281-
),
7282-
) as plugin:
7283-
await plugin._ensure_started()
7284-
assert plugin._resolved_location == "europe-west1"
7257+
When location is omitted from bigquery.Client(), client.query()
7258+
sends no location field in the API request, letting BigQuery
7259+
infer location from the referenced dataset. This prevents
7260+
silent view-creation failures for non-US datasets.
7261+
"""
72857262

72867263
@pytest.mark.asyncio
7287-
async def test_location_falls_back_on_get_dataset_failure(
7264+
async def test_client_created_without_location(
72887265
self,
72897266
mock_auth_default,
72907267
mock_to_arrow_schema,
72917268
mock_asyncio_to_thread,
72927269
):
7293-
"""Falls back to configured location when get_dataset fails."""
7270+
"""bigquery.Client is created without a location parameter."""
72947271
with mock.patch.object(bigquery, "Client", autospec=True) as mock_bq_cls:
7295-
mock_bq_cls.return_value.get_dataset.side_effect = Exception("not found")
72967272
mock_bq_cls.return_value.get_table.side_effect = (
72977273
cloud_exceptions.NotFound("table")
72987274
)
@@ -7302,28 +7278,27 @@ async def test_location_falls_back_on_get_dataset_failure(
73027278
project_id=PROJECT_ID,
73037279
dataset_id=DATASET_ID,
73047280
table_id=TABLE_ID,
7305-
location="asia-northeast1",
7281+
location="europe-west1",
73067282
config=bigquery_agent_analytics_plugin.BigQueryLoggerConfig(
73077283
create_views=False,
73087284
),
73097285
) as plugin:
73107286
await plugin._ensure_started()
7311-
assert plugin._resolved_location == "asia-northeast1"
7287+
7288+
mock_bq_cls.assert_called_once()
7289+
_, kwargs = mock_bq_cls.call_args
7290+
assert "location" not in kwargs
73127291

73137292
@pytest.mark.asyncio
7314-
async def test_views_created_with_resolved_location(
7293+
async def test_view_query_omits_location(
73157294
self,
73167295
mock_auth_default,
73177296
mock_to_arrow_schema,
73187297
mock_asyncio_to_thread,
73197298
):
7320-
"""View creation DDL passes resolved location to client.query()."""
7321-
mock_dataset = mock.MagicMock()
7322-
mock_dataset.location = "europe-west1"
7323-
7299+
"""View creation DDL queries do not pass an explicit location."""
73247300
with mock.patch.object(bigquery, "Client", autospec=True) as mock_bq_cls:
73257301
mock_client = mock_bq_cls.return_value
7326-
mock_client.get_dataset.return_value = mock_dataset
73277302
mock_client.get_table.return_value = mock.MagicMock()
73287303
mock_client.query.return_value.result.return_value = None
73297304

@@ -7337,11 +7312,11 @@ async def test_views_created_with_resolved_location(
73377312
) as plugin:
73387313
await plugin._ensure_started()
73397314

7340-
# Verify query() was called with location kwarg
73417315
assert mock_client.query.call_count > 0
73427316
for call in mock_client.query.call_args_list:
73437317
_, kwargs = call
7344-
assert kwargs.get("location") == "europe-west1"
7318+
# No explicit location — BQ infers from dataset
7319+
assert "location" not in kwargs
73457320

73467321
@pytest.mark.asyncio
73477322
async def test_view_error_still_logged(
@@ -7351,12 +7326,8 @@ async def test_view_error_still_logged(
73517326
mock_asyncio_to_thread,
73527327
):
73537328
"""View creation errors are logged but not raised."""
7354-
mock_dataset = mock.MagicMock()
7355-
mock_dataset.location = "US"
7356-
73577329
with mock.patch.object(bigquery, "Client", autospec=True) as mock_bq_cls:
73587330
mock_client = mock_bq_cls.return_value
7359-
mock_client.get_dataset.return_value = mock_dataset
73607331
mock_client.get_table.return_value = mock.MagicMock()
73617332
mock_client.query.return_value.result.side_effect = Exception(
73627333
"view error"

0 commit comments

Comments
 (0)