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

Add response status to response object #56

Closed
grantr opened this issue Apr 9, 2014 · 3 comments
Closed

Add response status to response object #56

grantr opened this issue Apr 9, 2014 · 3 comments

Comments

@grantr
Copy link
Contributor

grantr commented Apr 9, 2014

The Elasticsearch 1.0 api is removing the ok attributes that have historically been used to detect successful responses. In several of these apis it is unclear how to detect a successful response from the response body.

I've resisted this so far, but I wonder if now might be the time to start returning a Response object instead of the raw response hash. My initial vision for this is as simple as possible:

class Response < Hash
  attr_accessor :status, :headers
end

This would allow api consumers to do things like:

response = docs.index(document)
if response.status != 201
   # fail
end

I'd avoid adding abstractions like success? unless there was a clear benefit from having them in the low level client. IMO those kinds of things should be at a higher level (even though we don't have a higher level yet). The Response object is necessary in the client because the information would otherwise be unretrievable.

/cc @github/search

@grantr
Copy link
Contributor Author

grantr commented Apr 9, 2014

While we may discover a way to detect success or failure from the response, I think the Elasticsearch devs feel that the response status is the canonical way to detect success or failure. See elastic/elasticsearch#4310

@grantr
Copy link
Contributor Author

grantr commented Apr 9, 2014

One counterpoint to this is our existing error handling in client.rb. That may already offer all the failure detection necessary.

@grantr
Copy link
Contributor Author

grantr commented Jun 18, 2014

Closing. I think the existing error handling is enough for now.

@grantr grantr closed this as completed Jun 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant