-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Remove] Type from Percolate query API #2490
[Remove] Type from Percolate query API #2490
Conversation
indexedDocumentType = in.readOptionalString(); | ||
if (in.getVersion().before(Version.V_2_0_0)) { | ||
String indexedDocumentType = in.readOptionalString(); | ||
assert indexedDocumentType == null; |
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 assertion needed? readOptionalString
assumes null
value but not necessarily
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 @reta for the comment. Typeless version of this query in 7.x is represented by a null documentType and indexedDocumentType.
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.
Instead of assert do we want to throw IllegalArgumentException
since assertions could be disabled? (this is why we did it in other PRs so it would be consistent with those serialization methods)
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 @nknize for the comment. I will replace assertion with throw IllegalArgumentException
.
0741191
to
4a7618a
Compare
✅ Gradle Check success 074119148fcde0afe467806f39559cf58870e148 |
✅ Gradle Check success 4a7618a95382fb8f4669a74eda372d41edaff633 |
Signed-off-by: Suraj Singh <[email protected]>
Signed-off-by: Suraj Singh <[email protected]>
4a7618a
to
d61b06a
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.
Looks good! 🎉
Signed-off-by: Suraj Singh [email protected]
Description
Remove types from percolate query API
Related: #1940
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.