Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Feb 17, 2017

This pull request adds the fromXContent() parsing method to BulkResponse.

This commit adds the `fromXContent()` parsing method to BulkResponse.
@tlrx tlrx force-pushed the add-parsing-method-to-bulk-response branch from f9042ea to a570fd3 Compare February 20, 2017 09:00
@tlrx tlrx added review and removed WIP labels Feb 20, 2017
@tlrx tlrx changed the title [WIP] Add parsing method to bulk response Add parsing method to bulk response Feb 20, 2017
@tlrx tlrx requested review from cbuescher and javanna February 20, 2017 09:01
@tlrx
Copy link
Member Author

tlrx commented Feb 20, 2017

This is ready to review.

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM

XContentParser.Token token = parser.nextToken();
ensureExpectedToken(XContentParser.Token.START_OBJECT, token, parser::getTokenLocation);

long took = 0;
Copy link
Member

Choose a reason for hiding this comment

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

nit: shall we initialize it to -1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would be aligned with ingest took millis.

} else if (INGEST_TOOK.equals(currentFieldName)) {
ingestTook = parser.longValue();
} else if (ERRORS.equals(currentFieldName) == false) {
throwUnknownField(currentFieldName, parser.getTokenLocation());
Copy link
Member

Choose a reason for hiding this comment

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

we don't parse errors as we don't have a field for it in the response?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, errors is the output of the BulkResponse.hasFailures() method which simply iterates over bulk item and checks for failed item.

@tlrx tlrx merged commit 39ed76c into elastic:master Feb 21, 2017
@tlrx tlrx deleted the add-parsing-method-to-bulk-response branch February 21, 2017 09:49
@tlrx
Copy link
Member Author

tlrx commented Feb 21, 2017

Thanks @javanna !

tlrx added a commit that referenced this pull request Feb 21, 2017
This commit adds the `fromXContent()` parsing method to BulkResponse.
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