Skip to content

fix(bigframes): resolve session-scoped API method logging - #18076

Open
shuoweil wants to merge 6 commits into
mainfrom
shuowei-fix-session-logging-and-labels
Open

fix(bigframes): resolve session-scoped API method logging#18076
shuoweil wants to merge 6 commits into
mainfrom
shuowei-fix-session-logging-and-labels

Conversation

@shuoweil

@shuoweil shuoweil commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Fixes an issue where API method calls on DataFrame and Series objects were recorded into global state instead of their specific session, preventing cross-session and cross-test logging interference.

  • Updated log_adapter._find_session to detect active sessions directly from DataFrame and Series instances.
  • Added recursion safeguards during session property lookup in log_adapter.

Fixes #<545233537> 🦕

@shuoweil
shuoweil requested review from sycai and tswast August 11, 2026 19:29
@shuoweil shuoweil self-assigned this Aug 11, 2026
@shuoweil
shuoweil requested review from a team as code owners August 11, 2026 19:29

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the session resolution logic in log_adapter.py to search through all positional arguments for a Session instance or objects containing a _session or session attribute. It also updates the unit tests in test_io_bigquery.py to use explicit mock sessions and log adapter calls instead of relying on DataFrame operations. The review feedback suggests refactoring the session attribute lookup in _find_session to eliminate duplicated code by iterating over a tuple of attribute names.

Comment thread packages/bigframes/bigframes/core/logging/log_adapter.py Outdated
@shuoweil
shuoweil force-pushed the shuowei-fix-session-logging-and-labels branch from 0f0d41f to 74e2ba2 Compare August 11, 2026 19:32
@shuoweil
shuoweil marked this pull request as draft August 11, 2026 19:32
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@shuoweil
shuoweil marked this pull request as ready for review August 11, 2026 19:36
@shuoweil shuoweil changed the title fix: resolve session-scoped api logging and stabilize label trimming tests fix: resolve session-scoped API method logging Aug 11, 2026
@daniel-sanche daniel-sanche changed the title fix: resolve session-scoped API method logging fix(bigframes): resolve session-scoped API method logging Aug 11, 2026
Comment on lines +330 to +337
for arg in args:
if isinstance(arg, Session) and _is_session_initialized(arg):
return arg
if hasattr(arg, "__dict__") and "_block" in arg.__dict__:
session = getattr(arg, "_session", None)
if isinstance(session, Session) and _is_session_initialized(session):
return session

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be place this before the kwargs check to preserve the original checking sequence?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Reordered _find_session to check args before kwargs, preserving the original sequence.

@shuoweil
shuoweil enabled auto-merge (squash) August 11, 2026 22:31
@shuoweil shuoweil added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 11, 2026
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 11, 2026
@shuoweil shuoweil added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kokoro:force-run Add this label to force Kokoro to re-run the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants