-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Cut AnalyzeResponse over to Writeable #41915
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
|
Pinging @elastic/es-search |
|
I started to look at AnalysisRequest as well, but that's going to be a bigger change, partially because it's used in the HLRC and so making things immutable will be a breaking change for clients. |
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.
should we make the fields name and texts final ?
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.
++
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.
should we make name and tokens fields to be final?
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.
++
1aaa532 to
6e7c8e4
Compare
|
@elasticmachine run elasticsearch-ci/docbldesx |
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 @romseygeek
| @Deprecated | ||
| protected abstract Response newResponse(); | ||
|
|
||
| protected Writeable.Reader<Response> getResponseReader() { |
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 for this @romseygeek!
I tried my hand at a follow-up to remove the adapter for this here #41985.
This commit makes AnalyzeResponse and its various helper classes implement Writeable. The classes are also now immutable. Relates to #34389
This commit makes AnalyzeResponse and its various helper classes implement Writeable. The classes are also now immutable. Relates to elastic#34389
This commit makes AnalyzeResponse and its various helper classes implement
Writeable. The classes are also now immutable.
Relates to #34389