-
Notifications
You must be signed in to change notification settings - Fork 258
fix: fix https://github.com/langgenius/dify-plugin-daemon/issues/406 #502
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 @fatelei, 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 resolves a critical issue where the plugin daemon would experience connection failures following an ECS task redeployment. The problem stemmed from the daemon attempting to connect to stale, non-existent IP addresses cached in Redis. The solution introduces a Highlights
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
|
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 effectively addresses the issue of connection failures after ECS redeployment by introducing a ClusterDisabled configuration flag and enhancing the request redirection logic. The changes include redirecting to localhost for the current node and implementing a retry mechanism with IP cache refreshing for other nodes, which significantly improves resilience. While the core logic is sound, I've identified several areas for improvement, particularly in the test suite's quality and some opportunities to reduce code duplication and simplify control flow for better maintainability. My detailed comments are below.
|
Hi, A huge refactor has been committed to main, would you mind rebase to main to solve the conflicts? |
done |
|
@Yeuoly I didn't find any explicit logical errors in this PR and it can pass |
Description
fix #406
Plugin daemon fails after ECS task redeployment with connection errors due to stale IP addresses cached in Redis. When containers get new IP
addresses during redeployment, the cluster system continues trying to connect to old, non-existent IP addresses.
Type of Change
Essential Checklist
Testing
Bug Fix (if applicable)
Fixes #123orCloses #123)Additional Information
Please provide any additional context that would help reviewers understand the changes.