Skip to content
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

Return an HTTP error code when a suggest request failed instead of 200 #10104

Conversation

obourgain
Copy link
Contributor

The _suggest API always returns 200 regardless of the call being successful or not.
This PR sets the http status in a way similar to the SearchResponse https://github.com/elastic/elasticsearch/blob/master/src/main/java/org/elasticsearch/action/search/SearchResponse.java#L71

@s1monw
Copy link
Contributor

s1monw commented Mar 18, 2015

I like this change, can we instead of duplicating the functionality add a static method to RestStatus that looks like this public static RestStatus status(int successfulShards, int totalShards, ShardOperationFailedException... failures) and use it in both places?

@s1monw s1monw added >bug v2.0.0-beta1 :Search/Suggesters "Did you mean" and suggestions as you type v1.6.0 labels Mar 18, 2015
@s1monw
Copy link
Contributor

s1monw commented Mar 18, 2015

@areek I assigned this to you, can you take care of it?

@obourgain
Copy link
Contributor Author

With a quick search, it seems that there are some other place that have a similar loop over failures and could also use a static method.

@s1monw
Copy link
Contributor

s1monw commented Mar 18, 2015

With a quick search, it seems that there are some other place that have a similar loop over failures and could also use a static method.

cool, if you could update your PR that would be awesome!

@obourgain obourgain force-pushed the suggest_returns_400_for_invalid_request branch from 16227aa to b83036b Compare March 18, 2015 21:03
@obourgain
Copy link
Contributor Author

I updated it with the status() method on RestStatus as suggested.

CountResponse, RestSuggestAction, SearchResponse and SnapshotInfo will use this method.

For the record, there are also loops over failures in SearchPhaseExecutionException and DeleteByQueryResponse but they are quite different so I didn't try to modify those.

@areek
Copy link
Contributor

areek commented Mar 19, 2015

@obourgain Thanks for the the PR! This looks good to me. I will merge this in early next week

@s1monw
Copy link
Contributor

s1monw commented Mar 19, 2015

@areek it's pretty low risk and is a bug, should we pull it for 1.5?

@areek
Copy link
Contributor

areek commented Mar 19, 2015

@s1monw makes sense, will merge it in

@areek
Copy link
Contributor

areek commented Mar 19, 2015

Merged to 1.5 (47d0ae6),
1.x (35fcdf5) and master (deade2e)

@areek areek closed this Mar 19, 2015
@areek areek added the v1.5.0 label Mar 19, 2015
@clintongormley clintongormley changed the title return an HTTP error code when a suggest request failed instead of 200 Return an HTTP error code when a suggest request failed instead of 200 Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Suggesters "Did you mean" and suggestions as you type v1.5.0 v1.6.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants