-
Notifications
You must be signed in to change notification settings - Fork 115
Shard Failure alternatives support #5178
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
Conversation
|
Following you can find the validation changes against the target branch for the APIs. No changes detected. You can validate these APIs yourself by using the |
|
@Anaethelion, @flobernd, @miguelgrinberg checking if this breaks any generator before merging |
|
@l-trotta the Python DSL generator ignores the aliases, so this change causes no problems for me. |
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.
Thanks! The change itself LGTM. We still need to wait confirmation for .NET and Go.
|
@l-trotta This will only translates into alternatives field names within deserialization in Go, so you're good. 👍 |
|
Since I suggested the use of aliases, LGTM from my side as well 😅 I'm handling it the same way as Go. |
(cherry picked from commit 168e405)
(cherry picked from commit 168e405)
(cherry picked from commit 168e405)
(cherry picked from commit 168e405)
(cherry picked from commit 168e405)
Originally reported in elastic/elasticsearch-java#408.
ShardFailure has 3 variants in the server code:
shardis nullable and there'sstatus), and it's used by Scrollable responsesprimary, and is used by every response that extends ReplicationResponse (mainly Update, Delete and Index, but also some Bulk)To support all 3 without creating specific classes and introduce breaking changes we have decided to have ShardFailure support all variants by using aliases and making additional fields nullable (with
reasonbeing the only non optional field that every variant supports)