fix: show progress even in job optional queries #2119

tswast · 2025-09-25T22:02:02Z

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
Ensure the tests and linter pass
Code coverage does not decrease (if any source code was changed)
Appropriate docs were updated (if necessary)

Fixes b/409390651
Towards b/409104302
🦕

review-notebook-app · 2025-09-25T22:02:07Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

* feat: Render more BigQuery events in progress bar This change updates bigframes/formatting_helpers.py to render more event types from bigframes/core/events.py. Specifically, it adds rendering support for: - BigQueryRetryEvent - BigQueryReceivedEvent - BigQueryFinishedEvent - BigQueryUnknownEvent This provides users with more detailed feedback during query execution in both notebook (HTML) and terminal (plaintext) environments. * feat: Render more BigQuery events in progress bar This change updates bigframes/formatting_helpers.py to render more event types from bigframes/core/events.py. Specifically, it adds rendering support for: - BigQueryRetryEvent - BigQueryReceivedEvent - BigQueryFinishedEvent - BigQueryUnknownEvent This provides users with more detailed feedback during query execution in both notebook (HTML) and terminal (plaintext) environments. Unit tests have been added to verify the rendering of each new event type. --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

tswast · 2025-09-26T19:47:10Z

Tested manually with several ways of launching queries. All of which show some output now:

TrevorBergeron · 2025-09-26T20:11:09Z

bigframes/core/events.py

+class ExecutionStarted(Event):
+    pass


Should we have an execution_id or similar so we can correlate all the events tied to a single request?

That could help if we start doing async / background query execution. I don't think it's needed right now, though.

TrevorBergeron · 2025-09-26T20:13:31Z

bigframes/core/events.py

+
+@dataclasses.dataclass(frozen=True)
+class Subscriber:
+    callback_ref: weakref.ref


I think its more intuitive to just keep subscribers alive? What is the scenario we are imagining? I could imagine this ref could deleted even when subscriber is still alive, because they created an ephemeral function that went out of scope, though the target of said function is still around.

What is the scenario we are imagining?

The anywidget table widgets. When they re-run the cell, the TableWidget we create is no longer needed and should go out of scope, but as far as I know, we don't really have an opportunity to unsubscribe at that time.

I really do think the weakref here will have quite unintuitive results. I think there are some other options, basically the main thing is we want to unsubscribe before the callback becomes invalid (because it points at resources that no longer exist, most crucially). The subscriber itself should be able to time this best, and it may not be quite correlated with python object cleanup.

Hmmm, maybe any widget could make some short lived subscribers before/after a call to to_pandas_batches()? I can give it a try.

TrevorBergeron · 2025-09-26T20:14:21Z

bigframes/core/events.py

+            callback(event)
+
+
+publisher = Publisher()


Do we absolutely need global state? And if we do need it, can it at least be passed into sessions as a field?

I suspect we don't have a need for it right now, as we've moved the cell-level visualization to bigframes anywidget mode. I can try to refactor.

Moved to Session-level in

move publisher to session 0008e99

and

fix remaining mypy errors 1cf0dfd

TrevorBergeron · 2025-09-26T21:17:27Z

bigframes/core/events.py

+        self._subscribers: List[Subscriber] = []
+        self._subscribers_lock = threading.Lock()
+
+    def subscribe(self, callback):


Should subscribers specify what they are subscribing too in terms of event type enum or class? Otherwise new event types will change the behavior of existing subcribers

I don't see a need for this right now. The only purpose so far is progress bars, which should receive everything.

Yeah, I guess as long as this is internal-use only, we can tack on features like this later, and refactor callers as needed.

TrevorBergeron · 2025-09-26T21:19:40Z

bigframes/core/events.py

+class Publisher:
+    def __init__(self):
+        self._subscribers: List[Subscriber] = []
+        self._subscribers_lock = threading.Lock()


Single lock on a global could cause unexpected cross-session contention.

tswast · 2025-10-06T14:12:35Z

e2e failure: FAILED tests/system/large/blob/test_function.py::test_blob_transcribe[gemini-2.0-flash-lite-001-False] appears to be unrelated

TrevorBergeron · 2025-10-03T20:18:21Z

bigframes/session/anonymous_dataset.py

                self._datset_ref = bf_io_bigquery.create_bq_dataset_reference(
                    self.bqclient,
                    location=self._location,
+                    publisher=self._publisher,


Why here but no publishing in eg create_temp_table? How do we determine what types of jobs we are publishing for?

The real answer is that create_temp_table isn't calling start_query_with_client, so it didn't show up in my refactor. As discussed in chat, it also breaks some of the assumptions about a query being associated with an execution.

In the interest of transparency, we probably should be notifying the user when we run such queries, though. I'll do a search for client.query / client.query_and_wait and see what I find.

TrevorBergeron · 2025-10-03T20:26:52Z

bigframes/session/bq_caching_executor.py

        array_value: bigframes.core.ArrayValue,
        execution_spec: ex_spec.ExecutionSpec,
    ) -> executor.ExecuteResult:
+        self._publisher.publish(bigframes.core.events.ExecutionStarted())


No action now, but maybe we just end up doing a decorator/wrapper for the execute method if we want to generalize across executor types?

TrevorBergeron · 2025-10-06T19:48:40Z

bigframes/core/events.py

+        return subscriber
+
+    def unsubscribe(self, subscriber: Subscriber):
+        self._subscribers.remove(subscriber)


Might cause errors if done concurrently with iteration?

TrevorBergeron · 2025-10-06T19:49:47Z

bigframes/core/events.py

+    def __init__(self, callback: Callable[[Event], None], *, publisher: Publisher):
+        self._publisher = publisher
+        self._callback = callback
+        self._subscriber_id = str(uuid.uuid4())


do we even need to convert to string?

Looks like we don't. It's hashable without use of UUID.

In [1]: import uuid In [2]: type(uuid.uuid4()) Out[2]: uuid.UUID In [3]: hash(uuid.uuid4()) Out[3]: 695624583744666396

TrevorBergeron · 2025-10-06T19:51:17Z

bigframes/core/events.py

+        return value._subscriber_id == self._subscriber_id
+
+    def close(self):
+        self._publisher.unsubscribe(self)


Another alternative to explicitly removing from subscriber list in a blocking way is to just flag oneself as closed, and the publisher can then remove at its convenience. I think this approach works fine though

I'm wary of that because it would mean a circular reference would hang around until the next time an event is published, but I suppose that'd be OK.

tswast added 2 commits September 25, 2025 21:04

fix: show progress even in job optional queries

1eccb3a

first attempt at publisher

d03e5d1

product-auto-label bot added the size: l Pull request size is large. label Sep 25, 2025

product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. label Sep 25, 2025

tswast and others added 4 commits September 26, 2025 15:07

Merge remote-tracking branch 'origin/main' into b409390651-progress-bar

e6c3ba9

report execution started/stopped in read_gbq_query

70d8324

render bigquery sent events

5b4b250

product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Sep 26, 2025

fix job links

4d9f37a

tswast marked this pull request as ready for review September 26, 2025 18:06

tswast requested review from a team as code owners September 26, 2025 18:06

tswast requested a review from shuoweil September 26, 2025 18:06

blunderbuss-gcf bot assigned GarrettWu Sep 26, 2025

tswast added 3 commits September 26, 2025 19:08

fix system tests

fc1e630

fix mypy

d1a7f70

fix unit tests

253de65

tswast requested review from TrevorBergeron and removed request for shuoweil September 26, 2025 19:39

support more event types

5fec058

TrevorBergeron reviewed Sep 26, 2025

View reviewed changes

tswast added 4 commits September 29, 2025 16:27

Merge remote-tracking branch 'origin/main' into b409390651-progress-bar

91506c3

move publisher to session

0008e99

fix remaining mypy errors

1cf0dfd

update text

a6600f8

GarrettWu removed their assignment Sep 30, 2025

tswast added 4 commits October 2, 2025 20:46

Merge remote-tracking branch 'origin/main' into b409390651-progress-bar

51e0ca6

add explicit unsubscribe

b35015e

Merge remote-tracking branch 'origin/main' into b409390651-progress-bar

55cc2f7

fix presubmits

e7ca461

TrevorBergeron reviewed Oct 6, 2025

View reviewed changes

tswast added 2 commits October 6, 2025 20:42

Merge remote-tracking branch 'origin/main' into b409390651-progress-bar

0d0ad68

add lock for publisher and publish temp table creations

7edbb0a

TrevorBergeron approved these changes Oct 7, 2025

View reviewed changes

tswast merged commit 1f48d3a into main Oct 8, 2025
19 of 25 checks passed

tswast deleted the b409390651-progress-bar branch October 8, 2025 16:04

release-please bot mentioned this pull request Oct 8, 2025

chore(main): release 2.25.0 #2145

Merged

fix: show progress even in job optional queries #2119

fix: show progress even in job optional queries #2119

Uh oh!

Conversation

tswast commented Sep 25, 2025

Uh oh!

review-notebook-app bot commented Sep 25, 2025

Uh oh!

tswast commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tswast commented Oct 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants