Skip to content

Conversation

@jroth1111
Copy link

Supersedes #665. Docs-only updates; routing example consolidated in operations guide; no config.example.yaml changes.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @jroth1111, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on enhancing the project's documentation by introducing a detailed operations guide. This new guide consolidates information on various advanced operational features such as secret management, robust configuration parsing, encrypted credential storage, comprehensive metrics, intelligent caching, and sophisticated routing and failover mechanisms. The README.md has also been updated to reflect these new capabilities, ensuring users can quickly grasp the extensive functionalities offered by the proxy.

Highlights

  • New Operations Guide: A comprehensive docs/operations.md file has been added, detailing advanced operational features of the proxy, including security, observability, and routing strategies.
  • README.md Update: The README.md has been significantly expanded to include new sections on 'Operational Enhancements' and 'Health-Based Routing & Smart Load Balancing', providing a high-level overview of the advanced capabilities and linking to the new operations guide.
  • Routing Strategy Clarification: Minor documentation updates were made across SDK usage guides (docs/sdk-advanced.md, docs/sdk-advanced_CN.md, docs/sdk-usage.md, docs/sdk-usage_CN.md) to clarify that a nil selector in coreauth.NewManager defaults to the 'fill-first' selection strategy.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@jroth1111
Copy link
Author

@luispater replacement for #665. Docs-only (README/docs). Routing example consolidated; duplicate block removed. Ready for re-review.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant documentation updates, including a new operations guide and enhancements to the README. The changes provide a comprehensive overview of new operational features like health-based routing, rate limiting, and encrypted storage.

My review focuses on ensuring the accuracy of the new documentation. I've found a few inconsistencies regarding the default routing strategy. The documentation states fill-first is the default, but the codebase and example configuration indicate round-robin is the default. I've left several comments to correct this in the README, the new operations guide, and SDK usage examples. Correcting these will ensure developers have accurate information when using and configuring the proxy.

Overall, this is a great addition to the project's documentation.

README.md Outdated
Comment on lines 78 to 79
- **`fill-first`**: Drain one credential before moving to the next (default)
- **`round-robin`**: Sequential credential rotation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The documentation here indicates fill-first is the default strategy, but the code (sdk/cliproxy/builder.go) and config.example.yaml show that round-robin is the default. To ensure consistency, please update this section to reflect that round-robin is the default.

Suggested change
- **`fill-first`**: Drain one credential before moving to the next (default)
- **`round-robin`**: Sequential credential rotation
- **`fill-first`**: Drain one credential before moving to the next
- **`round-robin`**: Sequential credential rotation (default)


```yaml
routing:
strategy: "fill-first" # fill-first (default), round-robin, random, least-busy, lowest-latency
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The comment here states that fill-first is the default strategy. However, according to the implementation (sdk/cliproxy/builder.go) and config.example.yaml, the default strategy is round-robin. To maintain consistency and prevent confusion, please update the comment to reflect the correct default.

Suggested change
strategy: "fill-first" # fill-first (default), round-robin, random, least-busy, lowest-latency
strategy: "fill-first" # fill-first, round-robin (default), random, least-busy, lowest-latency

Register the executor with the core manager before starting the service:

```go
// nil selector uses the default "fill-first" selection strategy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment seems to be incorrect. Based on the implementation in sdk/cliproxy/auth/manager.go, when a nil selector is passed to coreauth.NewManager, it defaults to RoundRobinSelector, not fill-first. It would be great to correct this to avoid confusion for SDK users.

Suggested change
// nil selector uses the default "fill-first" selection strategy.
// nil selector uses the default "round-robin" selection strategy.

在启动服务前将执行器注册到核心管理器:

```go
// selector 传 nil 时默认使用 "fill-first" 选择策略。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment (in Chinese) incorrectly states that the default selection strategy is fill-first. According to the implementation in sdk/cliproxy/auth/manager.go, the default is RoundRobinSelector. Please correct this to avoid misleading users of the Chinese documentation.

Suggested change
// selector 传 nil 时默认使用 "fill-first" 选择策略。
// selector 传 nil 时默认使用 "round-robin" 选择策略。

The service uses a core `auth.Manager` for selection, execution, and auto‑refresh. When embedding, you can provide your own manager to customize transports or hooks:

```go
// nil selector uses the default "fill-first" selection strategy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This comment appears to be incorrect. The coreauth.NewManager function defaults to RoundRobinSelector when a nil selector is provided, as seen in sdk/cliproxy/auth/manager.go. Please update the comment to reflect the correct default behavior.

Suggested change
// nil selector uses the default "fill-first" selection strategy.
// nil selector uses the default "round-robin" selection strategy.

服务内部使用核心 `auth.Manager` 负责选择、执行、自动刷新。内嵌时可自定义其传输或钩子:

```go
// selector 传 nil 时默认使用 "fill-first" 选择策略。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the other SDK documents, this comment in the Chinese documentation is incorrect. The default selector is RoundRobinSelector, not fill-first. Please update this for consistency and accuracy.

Suggested change
// selector 传 nil 时默认使用 "fill-first" 选择策略。
// selector 传 nil 时默认使用 "round-robin" 选择策略。

@jroth1111
Copy link
Author

Addressed Gemini doc notes: updated README + operations + SDK docs (EN/CN) to state round-robin is the default when selector is nil; fill-first is no longer labeled default. Ready for re-review.

@jroth1111
Copy link
Author

Clarified fill-first intent in docs: now explicitly notes draining one account to rate limit/cooldown, then moving to the next to stagger rolling windows and sustain throughput. (Updated README + operations guide.)

@jroth1111
Copy link
Author

@luispater docs updates complete: routing default fixed to round-robin and fill-first intent clarified (drain to rate limit/cooldown to stagger rolling windows). CI green. Ready for re-review.

@jroth1111
Copy link
Author

Docs updates complete (round-robin default clarified, fill-first intent clarified). Ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant