go-elasticsearch icon indicating copy to clipboard operation
go-elasticsearch copied to clipboard

fasthttp doesn't support body for GET

Open arigon opened this issue 4 years ago • 8 comments

Hello

unfortunately, fasthttp does not support a body when using GET: https://github.com/valyala/fasthttp/issues/651

That's why fasthttp is not working with es.Search.WithBody(&buf)

Please add an option to switch the search request method to POST.

Alex

arigon avatar Nov 28 '19 20:11 arigon

Thanks for reporting the issue!

This is indeed something which needs to be solved. I have considered adding a global option to switch the HTTP method to POST when the body is not nil, similar to what other clients have been historically doing. But then I didn't want to make this change too invasive, and pollute the client configuration with another option. After all, this issue is related only to the valyala/fasthttp package at the moment, and the only way of using that package is to write a wrapper, as in the _examples/fasthttp/fasthttp.go example.

Therefore, I've updated that example wrapper, and also added a call to the Search API with body to the lightweight test for the example — have a look at the attached commit.

I think it's a good solution for the time being, and we can always revisit the global configuration option if it would seem useful for other use cases. What do you think?

karmi avatar Dec 04 '19 09:12 karmi

Well, it's highly unusual to use a body with GET. I would say a good solution without breaking anything would be this:

method = "GET"

if r.Body != nil {
		method = "POST"
}

I will try the fasthttp workaround.

UPDATE: the fasthttp workaround works for me. I would like to keep this issue open, because this topic needs to be adressed. In my opinion the library is incomplete regarding this matter because Elasticsearch offers to use POST for search request and the library does not. :)

arigon avatar Dec 04 '19 14:12 arigon

@arigon @karmi

I have another case where I need the Search API to work with POST. We use 3rd party service for storing logs. They have some sort of proxy to elastic search. And as a result, they don't support GET for the search API.

So IMHO this is must-have. Currently, I'll need either to fork or use custom transport :/

There is also this pr that's closed: https://github.com/elastic/go-elasticsearch/issues/41

spaiz avatar Apr 01 '20 21:04 spaiz

@spaiz you can use the fasthttp source where POST is forced

arigon avatar Apr 01 '20 21:04 arigon

@spaiz, understood. Most of the official clients support a configurable HTTP method for API calls with bodies — I'll look into adding that.

You don't have to fork, though. As @arigon says, you can use the fasthttp transport, or you can quite easily implement your own, with a logic like if req.Body != nil { req.Method = "POST" }. See https://github.com/elastic/go-elasticsearch/blob/master/_examples/customization.go#L21-L33

karmi avatar Apr 02 '20 03:04 karmi

@karmi Thas's exactly what I'm doing now.

api := req.Context().Value("api").(string) if api == "search" { req.Method = "POST" }

It's just would be nice to have it in the package. It looks like it can be implemented nicely just by adding a new optional like:

es.Search.WithMethod("POST"),

spaiz avatar Apr 02 '20 21:04 spaiz

I totally agree. That's why I decided to keep this issue open until an optional switcher has been added into the package. I hope @karmi will add it anytime into the package ;-)

arigon avatar Apr 02 '20 22:04 arigon

It looks like it can be implemented nicely just by adding a new optional (...)

There are other APIs which send body, and use GET by default, so it has to be a global configuration option. Fairly trivial to implement, but I'm deep in other things at the moment. I'll chew through to it at some point :)

karmi avatar Apr 03 '20 06:04 karmi