-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
[Streaming] Predictable order for results of /health/service/:serviceName to mimic memdb #9247
[Streaming] Predictable order for results of /health/service/:serviceName to mimic memdb #9247
Conversation
…Name to mimic memdb This ensures the result is consitent with/witout streaming Will partially fix hashicorp#9239
7be29a8
to
fb91190
Compare
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.
Thank you for the PR!
I agree this should fix the problem. Just a couple minor suggestions for the implementation.
@dnephin All done, thank you for the review |
6af4104
to
07ba1ff
Compare
* Renamed `cachedHealResultSorter` into `sortCheckServiceNodes` * Use `<` instead of `strings.Compare` * Single line comparison in unit test
07ba1ff
to
76d95fd
Compare
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.
Thank you! Looks great!
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/288647. |
🍒✅ Cherry pick of commit 08b8a92 onto |
…er_for_health [Streaming] Predictable order for results of /health/service/:serviceName to mimic memdb
This ensures the result is consitent with/witout streaming
Will partially fix #9239