-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
App Configuration Python Provider - Load Balancing #37692
base: main
Are you sure you want to change the base?
Conversation
… reshuffle even if nothing changed.
API change check API changes are not detected in this pull request. |
|
||
while self._replica_client_manager.has_next_client(): | ||
client = self._replica_client_manager.get_next_client() | ||
if not client: |
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.
Is this possible if has_next_client
is true?
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.
It shouldn't be, but the type checker throws an error if I don't check it. We could make client manager an iterable, but that could cause it's own issues.
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.
I agree, I would rather we did not make the manager iterable.
So essentially, no matter how we write this, we need the null check?
Even if we put the client check as the sentinel expression for the while loop?
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 issue is that we need to know the last client used. Most ways like getting the clients and then using them like we did perviously result in us not knowing which one we ended with. Or we would have to mess around on the end side of recording the last one used.
Thoughts?
Description
Adds optional load balancing, default is false.
If enabled a round robin of the config stores endpoints is done when refreshing configurations.
In addition, the order of the config store endpoints is randomized at start and when new ones are found so that multiple instances of the same application don't hit the same stores in the same order.
Also, some static type checking fixes were added from errors seen while updating.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines