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 types #75

Closed
jordansissel opened this issue Jul 30, 2019 · 3 comments
Closed

Add response types #75

jordansissel opened this issue Jul 30, 2019 · 3 comments

Comments

@jordansissel
Copy link

elasticsearch.Client.Get() returns *Response, error

Most responses in Elasticsearch will have at least a top-level predictable response structure. It'd be nice, for example, if Get() returned something like a GetResponse.

The workaround is roughly the following for every kind of response:

var getResponse struct {
  Index string `json:"_index"`
  ID string `json:"_id"`
  // a dozen other lines
}

err = json.NewDecoder(response.Body).Decode(&getResponse)
...
// do something with getResponse

Thoughts? Maybe I'm missing something in the docs, but it'd be very nice not to have to define response type structs all over the place.

@karmi
Copy link
Contributor

karmi commented Jul 31, 2019

So, the fundamental principle of the "low-level" client is that it doesn't "touch" the body, either for request in response; this is for multiple reasons:

  • To provide as much performance as possible, the body is not parsed, and only passed around as an io.Reader, and handling the body is left to the calling code.
  • To allow using any third-party JSON package, without declaring some kind of contract it has to implement.

Most responses in Elasticsearch will have at least a top-level predictable response structure.

That's a pretty brave statement :) In fact, Elasticsearch has quite a lot of completely different response structures, not only for different APIs, but also within a single API, like Search.

As of now, there is no formal specification of the request and response body structures — as we have for the API "surface" —, making it impractical (or impossible) to provide a reliable and maintainable implementation of the structure. (The maintainability of the olivere/elasticsearch Go package and the NEST client suffers due to this fact.)

With that in mind, you absolutely don't have to "define response type structs all over the place".

First of all, the package implements the String() method for responses, which makes it possible to pass it eg to fmt.Println, as you see in the README examples, in order to make one of the obvious use cases ("I want to see the response") as easy as possible. (There's also a built-in logging with multiple formats for that.)

Second, when it comes to the request body, the library provides a helper, esutil.JSONReader, so you can pass a custom struct with a document, or a map[string]interface{} with a query, to the client without messing around with conversion to io.Reader.

Third, in many cases, it's impractical to define the full representation of the Elasticsearch response, because you're interested only in specific parts. For this, the tidwall/gjson package is very convenient — and much faster.

Have you seen the examples in _examples/encoding? They may provide some inspiration about different approaches to decoding the response bodies.

@karmi
Copy link
Contributor

karmi commented Aug 2, 2019

Hello, any news here?

@karmi
Copy link
Contributor

karmi commented Nov 6, 2019

Hi @jordansissel, closing for now, please re-open if you want to discuss the topic more?

@karmi karmi closed this as completed Nov 6, 2019
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

2 participants