Skip to content

TermsLookup uses ObjectParser for x-content parsing#53733

Merged
romseygeek merged 1 commit intoelastic:masterfrom
romseygeek:refactoring/termslookup
Mar 19, 2020
Merged

TermsLookup uses ObjectParser for x-content parsing#53733
romseygeek merged 1 commit intoelastic:masterfrom
romseygeek:refactoring/termslookup

Conversation

@romseygeek
Copy link
Copy Markdown
Contributor

This commit refactors the fromXContent method in TermsLookup to use an
ObjectParser and adds an explicit parsing test.

Related to #53731

@romseygeek romseygeek added :Search/Search Search-related issues that do not fall into other categories >refactoring v8.0.0 labels Mar 18, 2020
@romseygeek romseygeek self-assigned this Mar 18, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-search (:Search/Search)

Copy link
Copy Markdown
Contributor

@mayya-sharipova mayya-sharipova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romseygeek Thanks, good refactoring, your PR LGTM.

Looks like index, id and path are required parameters for terms lookup. I wonder why our documentation states that they are optional. Should we correct this as well: https://www.elastic.co/guide/en/elasticsearch/reference/master/query-dsl-terms-query.html#query-dsl-terms-lookup-params

@romseygeek
Copy link
Copy Markdown
Contributor Author

Should we correct this as well

We should! I'll open a follow-up PR, thanks @mayya-sharipova

@romseygeek romseygeek merged commit 5c8cd16 into elastic:master Mar 19, 2020
@romseygeek romseygeek deleted the refactoring/termslookup branch March 19, 2020 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>refactoring :Search/Search Search-related issues that do not fall into other categories v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants