Harden Omni startup wait deadline - #74
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
/gemini review |
1 similar comment
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a constant omniStartupTimeout (5 minutes) and applies it to the startup wait strategies and overall deadline of the Omni container. It also adds a unit test to verify these timeout configurations. The feedback suggests replacing fragile reflection on the unexported deadline field of wait.MultiStrategy in the test with its public Timeout() method.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request introduces a dedicated omniStartupTimeout constant (set to 5 minutes) and applies it to both the individual wait strategies and the overall deadline in newOmni. A new test, TestNewOmniConfiguresStartupWaitTimeouts, is added to verify these timeout configurations. The review feedback recommends avoiding fragile reflection when asserting the deadline on wait.MultiStrategy by using its public Timeout() method instead, which also allows for the removal of the unused reflect import.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Configure the Omni wait multi-strategy with the public startup timeout default so the test no longer depends on unexported testcontainers internals while preserving the overall startup deadline. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
1 similar comment
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request configures explicit startup wait timeouts for the Spanner Omni container. It introduces a 5-minute startup timeout constant and applies it to the individual log and port wait strategies, as well as the combined multi-strategy. Additionally, a new unit test has been added to verify that these timeouts are correctly applied to the container request. There are no review comments, so I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Code Review
This pull request introduces a constant omniStartupTimeout of 5 minutes and applies it to the container startup wait strategies in omni.go. It also adds a new test TestNewOmniConfiguresStartupWaitTimeouts in omni_test.go to verify these timeout configurations. The feedback suggests removing the redundant custom containerCustomizerFunc type in favor of the library-provided testcontainers.CustomizeRequestOption, and using t.Context() instead of context.Background() in the test.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Replace the local test-only customizer adapter with testcontainers' public helper and tie the capture path to the test context in response to Gemini feedback. Co-authored-by: Cursor <cursoragent@cursor.com>
|
/gemini review |
1 similar comment
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
Code Review
This pull request introduces a defined startup timeout (omniStartupTimeout) of 5 minutes for the Spanner Omni container and configures it across the log, exposed port, and multi-strategy wait configurations. It also adds a comprehensive unit test to verify that these timeouts are correctly applied to the container request. I have no feedback to provide as the changes are well-implemented and include appropriate test coverage.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
1 participant
Summary
omniStartupTimeoutto the Omni log wait, exposed-port wait, and combined wait deadline.Test plan
go test ./... -run TestNewOmniConfiguresStartupWaitTimeoutsgo test ./...git diff --checkSPANEMUBOOST_ENABLE_OMNI_TESTSwas not set, so opt-in Docker/Omni integration tests were not run.Made with Cursor