-
Notifications
You must be signed in to change notification settings - Fork 35
Correct Vertex global region endpoint URL construction #84
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
Conversation
WalkthroughModified the VertexAI platform adapter's base URL construction to handle the "global" region differently. When region is "global", the base URL is now Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5–10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/GenerativeAI/Platforms/VertextPlatformAdapter.cs (1)
324-324: Update documentation to reflect the special case for the global region.The comment describes the URL pattern but doesn't mention that the "global" region is handled differently (the region prefix is omitted from the base domain).
Apply this diff to update the documentation:
/// <summary> /// Constructs the base URL for API requests with optional components. - /// Pattern: https://{region}-aiplatform.googleapis.com/{version}/projects/{projectId}/locations/{region}/publishers/{publisher} + /// Pattern: https://{region}-aiplatform.googleapis.com/{version}/projects/{projectId}/locations/{region}/publishers/{publisher} + /// Note: For the "global" region, the base URL is https://aiplatform.googleapis.com (region prefix is omitted). /// </summary>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/GenerativeAI/Platforms/VertextPlatformAdapter.cs(1 hunks)
🔇 Additional comments (1)
src/GenerativeAI/Platforms/VertextPlatformAdapter.cs (1)
345-345: Code correctly implements the global endpoint handling per Google Cloud API specifications.The line 345 implementation correctly handles the global endpoint by excluding the location from the endpoint name. The global endpoint URL follows the pattern https://aiplatform.googleapis.com/v1/projects/..., which is exactly what the code produces when
Region == "global".Google Cloud documentation specifies "global" (lowercase) as the location identifier, and the case-sensitive comparison aligns with this documented convention. The original suggestion to use case-insensitive comparison is not necessary—all tested regions in the codebase are lowercase, environment variable defaults use lowercase, and Google Cloud's API documentation consistently shows lowercase values only.
The fix correctly addresses the reported issue without requiring additional defensive measures.
Updated VertextPlatformAdapter.GetBaseUrl() to handle the global region as a special case without the region prefix.
Fixes #71
Tested with and without Microsoft.Extensions.AI
Summary by CodeRabbit