Skip to content

fix(bigquery-magics): wait for graph server startup before returning from init - #18763

Merged
parthea merged 6 commits into
mainfrom
wait-for-server-start
Oct 20, 2026
Merged

parthea merged 6 commits into
mainfrom
wait-for-server-start

Conversation

@parthea

@parthea parthea commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

GraphServer.init() starts the server on a background thread and returns immediately. If stop_server() is called right after init() (for example, in fast unit tests that don't make HTTP requests), self._server may still be None. When that happens, stop_server() does nothing and server_thread.join() hangs indefinitely in serve_forever().

This PR adds a threading.Event so init() waits until self._server is assigned before returning.

@parthea
parthea requested review from a team as code owners October 19, 2026 15:22
@parthea
parthea requested review from tswast and removed request for a team October 19, 2026 15:22

@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 introduces a synchronization mechanism using threading.Event to ensure that GraphServer.init() blocks until the background TCP server is fully initialized, resolving a race condition with stop_server(). A unit test was also added to verify this behavior. The feedback suggests avoiding an indefinite wait on self._server_ready.wait() by implementing a polling mechanism with a timeout, which prevents the main thread from hanging if the background server thread fails to start (e.g., due to port binding errors).

Comment thread packages/bigquery-magics/bigquery_magics/graph_server.py
parthea and others added 5 commits October 19, 2026 11:51
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

@tswast tswast 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.

Thanks!

@parthea
parthea merged commit 77e4d66 into main Oct 20, 2026
54 checks passed
@parthea
parthea deleted the wait-for-server-start branch October 20, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants