Migrates more ToXContentClasses #26321
Conversation
colings86
added
:Core/Infra/REST API
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
left a couple of comments, thanks for doing this!
| import static org.elasticsearch.common.xcontent.XContentParser.Token.END_OBJECT; | ||
|
|
||
| public final class InnerHitBuilder extends ToXContentToBytes implements Writeable { | ||
| public final class InnerHitBuilder implements Writeable, ToXContentObject { |
There was a problem hiding this comment.
should we add toString impl to this class now that it doesn't inherit anymore from ToXContentToBytes?
There was a problem hiding this comment.
this applies to all the classes that don't extend from ToXCOntentToBytes anymore and don't declare their own toString .
| // So we have a stack trace logged somewhere | ||
| return "{ \"error\" : \"" + ExceptionsHelper.detailedMessage(e) + "\"}"; | ||
| } | ||
| } |
There was a problem hiding this comment.
can we use some of the XContentHelper or Strings.toString here instead?
|
@javanna thanks for reviewing, I push two commits that address your comments, could you look again? |
| @Override | ||
| public String toString() { | ||
| try { | ||
| return XContentHelper.toXContent(this, XContentType.JSON, true).utf8ToString(); |
There was a problem hiding this comment.
Strings.toString already catches IOException. there is a variant of it where you can pass in pretty and himanReadable flags.
javanna
left a comment
javanna
left a comment
There was a problem hiding this comment.
LGTM besides the comment I left on using Strings.toString .
* More XContent migrations * Removes ToXContentToBytes * Adds toString to classes that used to extend ToXContentToBytes * use XContentHelper * more review comments * prettify tostring output
* More XContent migrations * Removes ToXContentToBytes * Adds toString to classes that used to extend ToXContentToBytes * use XContentHelper * more review comments * prettify tostring output
This reverts commit 4c6cb99.
|
Note that this PR is only in merged to master and 6.x as the changes are not compatible with 6.0 |
3 participants
This includes removing
ToXContentToBytes