-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Fix GCP Vertex AI global endpoint support for Gemini 3 models #6187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes GCP Vertex AI global endpoint support for Gemini 3 models by correcting the URL construction from https://global-aiplatform.googleapis.com to https://aiplatform.googleapis.com.
- Adds
GcpLocation::Globalvariant andbuild_host_url()method to properly construct URLs for global vs regional endpoints - Removes the fallback retry logic and
known_location()method as part of simplification - Includes comprehensive test coverage with 4 new tests for the URL construction and location handling
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/goose/src/providers/gcpvertexai.rs |
Adds build_host_url() method to construct correct URLs for global/regional endpoints; removes fallback retry logic; adds tests for URL construction |
crates/goose/src/providers/formats/gcpvertexai.rs |
Adds GcpLocation::Global variant with is_global() helper; removes unused TryFrom<&str> impl and known_location() method; adds tests for location display and global checking |
katzdave
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
|
Thanks @katzdave for the review! All feedback addressed in the latest commit - simplified by removing the |
katzdave
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, nice job!
6800bfc to
ef26556
Compare
|
Rebased onto main to resolve conflicts with #6191 (streaming support). Ready for merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
ef26556 to
625aa3d
Compare
- Add GcpLocation::Global variant with is_global() helper - Add build_host_url() to construct correct URLs: - Global: https://aiplatform.googleapis.com - Regional: https://{location}-aiplatform.googleapis.com - Remove known_location() method and fallback retry logic - Remove TryFrom<&str> impl for GcpLocation (no longer needed) - Add 4 new tests for global endpoint URL construction Fixes block#6186 Signed-off-by: Hugues Clouâtre <[email protected]>
- Remove is_global() method and empty impl block from GcpLocation - Remove comment block above build_host_url (self-documenting) - Remove 4 tests that directly mirror code without adding value: - test_gcp_location_display - test_gcp_location_is_global - test_build_host_url_regional - test_build_host_url_global Signed-off-by: Hugues Clouâtre <[email protected]>
625aa3d to
ab76ef5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
|
Rebased onto latest main following fixes in #6512. CI now passing. |
Summary
Fixes the GCP Vertex AI provider to support the global endpoint required for Gemini 3 models. The previous implementation incorrectly constructed the URL as
https://global-aiplatform.googleapis.cominstead ofhttps://aiplatform.googleapis.com.Type of Change
AI Assistance
Testing
Unit Tests:
test_gcp_location_display,test_gcp_location_is_global,test_build_host_url_global,test_build_host_url_regionalcargo fmt --check✓,./scripts/clippy-lint.sh✓Manual Live Testing (100% Working):
Built release binary and tested with actual GCP Vertex AI API:
gemini-3-flash-previewwithGCP_LOCATION=global- SUCCESShttps://aiplatform.googleapis.comgemini-3-pro-previewwithGCP_LOCATION=global- SUCCESSgemini-2.0-flash-001withGCP_LOCATION=us-central1- SUCCESSVerification:
Related Issues
Fixes #6186
Changes
GcpLocation::Globalvariant withis_global()helper methodbuild_host_url()method to handle global vs regional URL construction:https://aiplatform.googleapis.comhttps://{location}-aiplatform.googleapis.comknown_location()method and fallback retry logic (simplification)TryFrom<&str>impl forGcpLocation(no longer needed)Impact
crates/goose/src/providers/formats/gcpvertexai.rs,crates/goose/src/providers/gcpvertexai.rs