Skip to content

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

Open
parthea wants to merge 5 commits into
mainfrom
wait-for-server-start
Open

parthea wants to merge 5 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 Outdated
parthea and others added 4 commits October 19, 2026 11:51
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

1 participant