-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Cut over SearchResponse and SearchTemplateResponse to Writeable #41855
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
Cut over SearchResponse and SearchTemplateResponse to Writeable #41855
Conversation
|
Pinging @elastic/es-search |
| public SearchResponse() { | ||
| } | ||
|
|
||
| public SearchResponse(StreamInput in) throws IOException { |
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.
Can we also remove an empty constructor and make all member variables final here?
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.
yes this is not so hard so I am making this change.
| SearchTemplateResponse() { | ||
| } | ||
|
|
||
| SearchTemplateResponse(StreamInput in) throws IOException { |
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.
I wonder if we can remove an empty constructor and make all member variables final here? We may need to change a little fromXContent and have another constructor.
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.
this is possible yet it requires some work on how SearchTemplateResponse is used, as source and response are generally set after creation. I would prefer to keep this out of this PR.
mayya-sharipova
left a comment
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 @javanna! The PR looks good to me after we make changes SearchTemplateResponse.java and SearchResponse.java as outlined in the comments.
|
run elasticsearch-ci/bwc |
|
run elasticsearch-ci/docbldesx |
|
run elasticsearch-ci/1 |
Relates to #34389