Skip to content

Conversation

@dadoonet
Copy link
Contributor

This commit adds the parsing fromXContent() methods to the DeleteResponse class. The method is based on a ObjectParser because it is easier to use when parsing parent abstract classes like DocWriteResponse.

It also changes the ReplicationResponse.ShardInfo so that it now implements ToXContentObject. This way, the ShardInfo.fromXContent() method can be used by the DeleteResponse's ObjectParser.

Backport of #22680 in 5.x branch

This commit adds the parsing fromXContent() methods to the DeleteResponse class. The method is based on a ObjectParser because it is easier to use when parsing parent abstract classes like DocWriteResponse.

It also changes the ReplicationResponse.ShardInfo so that it now implements ToXContentObject. This way, the ShardInfo.fromXContent() method can be used by the DeleteResponse's ObjectParser.

Backport of elastic#22680 in 5.x branch
Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

Left two minor comments, otherwise LGTM

ShardId shardId = new ShardId(randomAsciiOfLength(5), randomAsciiOfLength(5), randomIntBetween(0, 5));
String type = randomAsciiOfLength(5);
String id = randomAsciiOfLength(5);
long seqNo = randomIntBetween(-2, 5);
Copy link
Member

Choose a reason for hiding this comment

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

Not needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OMG!


// We can't use equals() to compare the original and the parsed index response
// because the random index response can contain shard failures with exceptions,
// and those exceptions are not parsed back with the same types.
Copy link
Member

Choose a reason for hiding this comment

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

index -> delete

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I fixed that as well in master branch.

@dadoonet dadoonet merged commit b4851e8 into elastic:5.x Jan 20, 2017
@dadoonet dadoonet deleted the pr/delete-from-xcontent-5x branch January 20, 2017 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants