Fix Vertex channel global region format for claude models - #1253
Conversation
WalkthroughThe update modifies the URL construction logic in the Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
relay/channel/vertex/adaptor.go (1)
126-141: Factor out duplicate URL-building logic
Theglobal/ non-global split is now implemented three times (Gemini, new Claude branch, and previously elsewhere). Repeating thefmt.Sprintfblocks makes future maintenance error-prone.Consider extracting a small helper:
+func buildVertexURL(region, projectID, publisher, model, suffix string) string { + if region == "global" { + return fmt.Sprintf("https://aiplatform.googleapis.com/v1/projects/%s/locations/global/publishers/%s/models/%s:%s", + projectID, publisher, model, suffix) + } + return fmt.Sprintf("https://%s-aiplatform.googleapis.com/v1/projects/%s/locations/%s/publishers/%s/models/%s:%s", + region, projectID, region, publisher, model, suffix) +}Then call:
return buildVertexURL(region, adc.ProjectID, "anthropic", model, suffix), nilReduces duplication and keeps all publisher-specific URLs consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
relay/channel/vertex/adaptor.go(1 hunks)
🔇 Additional comments (1)
relay/channel/vertex/adaptor.go (1)
126-132: Correct handling of theglobalendpoint – looks good
The conditional branch introduces the correct base URL for theglobalregion, matching Google’s documented pattern (https://aiplatform.googleapis.com/...). Implementation aligns with the earlier Gemini logic.
| return fmt.Sprintf( | ||
| "https://%s-aiplatform.googleapis.com/v1/projects/%s/locations/%s/publishers/anthropic/models/%s:%s", | ||
| region, | ||
| adc.ProjectID, | ||
| region, | ||
| model, | ||
| suffix, | ||
| ), nil |
There was a problem hiding this comment.
Guard against empty region values
GetModelRegion can theoretically return an empty string for unknown models. In that case the non-global branch would render https://-aiplatform.googleapis.com/..., which is invalid and will 4xx.
Add a sanity check right after retrieving region, e.g.:
region := GetModelRegion(info.ApiVersion, info.OriginModelName)
+if region == "" {
+ return "", fmt.Errorf("unable to determine region for model %s", info.OriginModelName)
+}Prevents malformed hosts and surfaces configuration problems early.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In relay/channel/vertex/adaptor.go around lines 134 to 141, add a check
immediately after retrieving the region value from GetModelRegion to verify it
is not empty. If the region is empty, return an error indicating an invalid or
unknown region instead of proceeding to construct the URL. This prevents
generating malformed URLs like "https://-aiplatform.googleapis.com/..." and
surfaces configuration issues early.
Fix Vertex channel global region format for claude models
Fix Vertex channel global region format for claude models
Summary by CodeRabbit