-
Notifications
You must be signed in to change notification settings - Fork 10.2k
draft not for review. prototype showing retry messages #15448
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
Summary of ChangesHello @jacob314, 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 prototypes and implements the display of retry messages within the application's UI, providing immediate feedback to the user during transient errors or slow server responses. It involves integrating a retry counter into the UI state, visually updating loading indicators, and introducing robust simulation mechanisms for testing retry scenarios in various content generation services. Highlights
🧠 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 AssistThe 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
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 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
|
|
Size Change: +5.8 kB (+0.03%) Total Size: 22 MB
ℹ️ View Unchanged
|
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.
Code Review
This pull request introduces a UI feature to display retry attempts for network requests, which is a great enhancement for user experience by providing more transparency during transient network issues. The implementation involves adding a retryCount state that is propagated from the core retryWithBackoff utility up to the UI components. The LoadingIndicator is updated to show a warning message and change color during retries. The changes are well-structured, with updates to hooks, contexts, and components to support this new feature. Test utilities to simulate slow responses have also been added, which is useful for prototyping.
My main feedback is a high-severity concern regarding the increase of the default maximum retry attempts from 3 to 10. This could lead to users waiting for several minutes for the CLI to recover, which might be a frustrating experience.
|
|
||
| const DEFAULT_RETRY_OPTIONS: RetryOptions = { | ||
| maxAttempts: 3, | ||
| maxAttempts: 10, |
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.
The default maxAttempts for retries has been increased from 3 to 10. While this increases resilience to transient errors, it can also lead to a very long wait time for the user.
With an initial delay of 5s, a multiplier of 2, and a max delay of 30s, the total wait time for 9 retries could be over 3.5 minutes:
5s + 10s + 20s + 6 * 30s = 215s
This long duration might be perceived as the application being hung, leading to a poor user experience. Additionally, it could mask more persistent underlying network or service issues.
Consider whether this long retry duration is intended. A lower number of retries (e.g., 5) might offer a better balance between resilience and user experience. If a higher number is desired, it might be better to make it configurable by the user.
Summary
Details
Related Issues
How to Validate
Pre-Merge Checklist