feat: add user-agent delegation telemetry for pandas-gbq - #18184
feat: add user-agent delegation telemetry for pandas-gbq#18184shuoweil wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the to_dataframe method in table.py to append pandas-gbq telemetry to the client's user agent when delegation is supported, and adds comprehensive unit tests to verify this behavior. The review feedback suggests wrapping the user-agent update logic in a try-except block to ensure robust defensive programming and prevent potential crashes. Additionally, it recommends mocking installed_version in the geodataframe tests to avoid import errors in environments where pandas-gbq is not installed.
| if _versions_helpers.PANDAS_GBQ_VERSIONS.is_delegation_supported: | ||
| client_info = getattr( | ||
| getattr(self.client, "_connection", None), "_client_info", None | ||
| ) | ||
| if client_info: | ||
| ua = client_info.user_agent or "" | ||
| if "pandas-gbq" not in ua: | ||
| version = _versions_helpers.PANDAS_GBQ_VERSIONS.installed_version | ||
| client_info.user_agent = f"{ua} pandas-gbq/{version}".strip() |
There was a problem hiding this comment.
Telemetry and user-agent updates should be treated as best-effort operations. If client_info is a mock, a custom read-only object, or lacks the user_agent attribute, accessing or modifying client_info.user_agent directly could raise an AttributeError or other exceptions, causing the core to_dataframe() call to crash.
To ensure robust defensive programming, we should wrap this telemetry update in a try-except block and use getattr to safely access user_agent. Avoid silently passing on exceptions; instead, log the exception using logger.warning to aid in debugging and prevent masking underlying issues.
if _versions_helpers.PANDAS_GBQ_VERSIONS.is_delegation_supported:
try:
client_info = getattr(
getattr(self.client, "_connection", None), "_client_info", None
)
if client_info:
ua = getattr(client_info, "user_agent", None) or ""
if "pandas-gbq" not in ua:
version = _versions_helpers.PANDAS_GBQ_VERSIONS.installed_version
client_info.user_agent = f"{ua} pandas-gbq/{version}".strip()
except Exception as exc:
logger.warning("Failed to update telemetry user-agent: %s", exc)References
- Avoid broad
except Exception:blocks that silently returnNone. Instead, log the exception (e.g., usinglogger.warning) to aid in debugging and prevent masking underlying issues.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| ) | ||
| client_info.user_agent = f"{ua} pandas-gbq/{version}".strip() | ||
| except Exception as exc: | ||
| _LOGGER.warning("Failed to update telemetry user-agent: %s", exc) |
There was a problem hiding this comment.
Do we need this logging? I think it would print unwanted contents on the user's terminal or notebook, right?
|
|
||
| if _versions_helpers.PANDAS_GBQ_VERSIONS.is_delegation_supported: |
There was a problem hiding this comment.
Could you share the link to the code where pandas-gbq is used as the delegation?
Appends
pandas-gbq/<version>toClientInfo.user_agentwhenRowIterator.to_dataframe()orRowIterator.to_geodataframe()is executed and query delegation is supported bypandas-gbq.This PR enables backend metrics and BigQuery audit logs to differentiate between direct client-side conversion and delegated conversions, without altering existing DataFrame results or runtime behavior.
Supersedes #17704.
Key Changes
google/cloud/bigquery/table.py: Injectspandas-gbq/<version>intoclient_info.user_agentwith deduplication check whenPANDAS_GBQ_VERSIONS.is_delegation_supportedisTrue.tests/unit/test_table.py: Adds unit tests for user-agent injection, deduplication, missingclient_info/user_agent, and unsupported fallback behavior.Fixes #<540939659> 🦕