Conversation
|
Caution Review failedThe pull request is closed. """ WalkthroughThis update introduces comprehensive support for Gemini provider configuration, adding new keys and command-line flags for project, location, backend type, and API key. The Gemini client initialization logic is updated to use these settings, supporting both Gemini API and VertexAI backends with appropriate validation and option setters throughout the provider code. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Viper
participant Provider
participant GeminiClient
User->>CLI: Set Gemini config via flags
CLI->>Viper: Bind flags to config keys
User->>Provider: Request Gemini client
Provider->>Viper: Read Gemini config (api_key, backend, project, location)
Provider->>GeminiClient: Initialize with config options
GeminiClient-->>Provider: Client instance (validated)
Provider-->>User: Ready-to-use Gemini client
Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (1.64.8)Error: you are using a configuration file for golangci-lint v2 with golangci-lint v1: please use golangci-lint v2 Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
cmd/config_set.go (2)
36-40: Consider standardizing flag definition styleThe new Gemini flags are defined using
String()while most other flags in this file useStringP()which provides short-form aliases. Also, the naming uses dot notation directly in flag names, unlike other flags which use underscores and rely on viper bindings for the namespace.Consider updating for consistency:
- configSetCmd.Flags().String("gemini.project", "", availableKeys["gemini.project"]) - configSetCmd.Flags().String("gemini.location", "", availableKeys["gemini.location"]) - configSetCmd.Flags().String("gemini.backend", "BackendGeminiAPI", availableKeys["gemini.backend"]) - configSetCmd.Flags().String("gemini.api_key", "", availableKeys["gemini.api_key"]) + configSetCmd.Flags().StringP("gemini_project", "", "", availableKeys["gemini.project"]) + configSetCmd.Flags().StringP("gemini_location", "", "", availableKeys["gemini.location"]) + configSetCmd.Flags().StringP("gemini_backend", "", "BackendGeminiAPI", availableKeys["gemini.backend"]) + configSetCmd.Flags().StringP("gemini_api_key", "", "", availableKeys["gemini.api_key"])Then update the bindings accordingly.
39-39: Consider extracting "BackendGeminiAPI" as a constantThe string literal "BackendGeminiAPI" appears to be an enum value or constant. For better maintainability, consider referencing a defined constant.
- configSetCmd.Flags().String("gemini.backend", "BackendGeminiAPI", availableKeys["gemini.backend"]) + configSetCmd.Flags().String("gemini.backend", provider.BackendGeminiAPI, availableKeys["gemini.backend"])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmd/config_list.go(1 hunks)cmd/config_set.go(2 hunks)cmd/provider.go(1 hunks)provider/gemini/gemini.go(1 hunks)provider/gemini/options.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
cmd/provider.go (2)
provider/gemini/gemini.go (1)
New(143-194)provider/gemini/options.go (8)
WithToken(40-44)WithModel(47-51)WithMaxTokens(56-63)WithTemperature(69-76)WithTopP(79-83)WithBackend(118-129)WithProject(131-135)WithLocation(112-116)
🪛 GitHub Check: ubuntu-latest @ Go stable
provider/gemini/gemini.go
[failure] 163-163:
missing cases in switch of type genai.Backend: genai.BackendUnspecified (exhaustive)
🪛 GitHub Actions: Lint and Testing
provider/gemini/gemini.go
[error] 163-163: golangci-lint: missing cases in switch of type genai.Backend: genai.BackendUnspecified (exhaustive)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (go)
🔇 Additional comments (6)
cmd/provider.go (2)
38-41: Good implementation of fallback mechanism for API keysThe code now properly checks for a Gemini-specific API key first before falling back to the OpenAI key, which provides better isolation between provider configurations.
44-52: Backend configuration options added appropriatelyThe configuration now correctly passes Gemini-specific options including backend type, project, and location to support both VertexAI and GeminiAPI backends.
provider/gemini/options.go (3)
91-94: Backend configuration fields added appropriatelyThe configuration struct now properly includes fields for project, location, and backend to support the new configuration options.
97-105: Backend-specific validation logic looks goodThe validation correctly implements different requirements based on the selected backend, requiring project and location for VertexAI and API token for GeminiAPI.
144-144: Default backend selection looks goodSetting the default backend to
BackendGeminiAPIis appropriate for backward compatibility.cmd/config_set.go (1)
61-64: Bindings for new Gemini configuration look goodThe bindings correctly map the flag values to the corresponding Viper configuration keys, which aligns with the PR objective of enhancing Gemini provider configuration.
| func WithBackend(val string) Option { | ||
| return optionFunc(func(c *config) { | ||
| switch val { | ||
| case "BackendVertexAI": | ||
| c.backend = genai.BackendVertexAI | ||
| case "BackendGeminiAPI": | ||
| c.backend = genai.BackendGeminiAPI | ||
| default: | ||
| c.backend = genai.BackendGeminiAPI | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
Add handling for BackendUnspecified in WithBackend
Similar to the issue in gemini.go, the WithBackend function should handle the BackendUnspecified value to maintain consistency with the enumeration type.
func WithBackend(val string) Option {
return optionFunc(func(c *config) {
switch val {
case "BackendVertexAI":
c.backend = genai.BackendVertexAI
case "BackendGeminiAPI":
c.backend = genai.BackendGeminiAPI
+ case "BackendUnspecified":
+ c.backend = genai.BackendUnspecified
default:
c.backend = genai.BackendGeminiAPI
}
})
}Additionally, consider using string constants for these backend identifiers to avoid typos and improve maintainability.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func WithBackend(val string) Option { | |
| return optionFunc(func(c *config) { | |
| switch val { | |
| case "BackendVertexAI": | |
| c.backend = genai.BackendVertexAI | |
| case "BackendGeminiAPI": | |
| c.backend = genai.BackendGeminiAPI | |
| default: | |
| c.backend = genai.BackendGeminiAPI | |
| } | |
| }) | |
| } | |
| func WithBackend(val string) Option { | |
| return optionFunc(func(c *config) { | |
| switch val { | |
| case "BackendVertexAI": | |
| c.backend = genai.BackendVertexAI | |
| case "BackendGeminiAPI": | |
| c.backend = genai.BackendGeminiAPI | |
| case "BackendUnspecified": | |
| c.backend = genai.BackendUnspecified | |
| default: | |
| c.backend = genai.BackendGeminiAPI | |
| } | |
| }) | |
| } |
…pport - Add support for Gemini provider configuration, including project, location, backend, and API key options - Ensure gemini.api_key is treated as sensitive and hidden in config listing - Allow Gemini client to fall back to openai.api_key if gemini.api_key is not set - Pass Gemini backend, project, and location settings to the Gemini client - Refactor Gemini client initialization to support both VertexAI and GeminiAPI backends with appropriate configuration - Add validation for required project and location fields when using the VertexAI backend - Introduce new option functions: WithProject, WithLocation, and WithBackend for flexible Gemini configuration Signed-off-by: Bo-Yi Wu <[email protected]>
1 participant
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes