Initial refactoring for completion suggester#16398
Initial refactoring for completion suggester#16398areek merged 1 commit intoelastic:feature-suggest-refactoringfrom
Conversation
e87cc5b to
1eef734
Compare
There was a problem hiding this comment.
nit: Everything okay, but I found this (and the following same constructs) a little hard to read. Can I suggest writing the boolean flag first without conditions (e.g. out.writeBoolean(fuzzyOptions != null) and then have the if-block only? But either was is fine.
There was a problem hiding this comment.
Now we write the boolean flag first followed by an if-only block
|
@areek looks great, I left a few small comments but other than that tests look good and also pulling out all those separate classes makes things much easier to understand I think. |
6c5845f to
21c1da6
Compare
|
@cbuescher Thanks for the review. I addressed all your comments. |
There was a problem hiding this comment.
nit: can both new helper methods go somewhere further down in ESTestCase? From the comments it seems someone tries to keep some order here, and this would be the setup / cleanup section. Also maybe both should be public? Although protected also works, we need it in tests that subclass this anyway.
There was a problem hiding this comment.
Thanks for the suggestion, moved the helper methods to the right section
|
@areek thanks, left one minor comment and I'm getting a few test failures (StringDistanceImplTests, TermSuggestionBuilderTests, CompletionSuggesterBuilderTests), some of which might already be fixed on the feature branch. |
|
Thanks @cbuescher for looking, I think we should merge this in after investigating the test failures and implement parsing via innerFromXContent for CompletionSuggestionBuilder in a subsequent PR. |
0e8a3a1 to
2038429
Compare
This PR implements serialization methods for CompletionSuggestionBuilder based on the suggest refactoring work started in #16241 and improves test coverage. This simplifies CompletionSuggestionBuilder by making fuzzy, regex and context queries implement their own serialization methods.