net/context support

[rs]

Services could expose a DoC(ctx context.Context) method in addition to their Do command. When a context is passed, the underlaying HTTP request would handle the context's cancellation by using ctxhttp.

[olivere]

Yes, that seems like a very good idea. Maybe we should wait a bit until the dust settles about context.Context being added to net/http in 1.7 (maybe). I'm not up to date, but I remember there was a discussion about that recently.

[rs]

I see no sign of net/context being integrated into 1.7 unfortunately :/

[olivere]

It is discussed in various places by people from the core team. But you're right: Nothing is set in stone at this point in time.

[olivere]

Just two quick remarks:

1. I expect you want to use it for cancelable requests, correct? Depending on the implementation, that's quite some work as we have many endpoints. So I think I should focus on the ones that would benefit the most. Which are the endpoints you have in mind? Search, I suppose. Anything else?
2. If you want search timeouts, there's already a way. ES supports a timeout parameter that is exposed e.g. in the Search service and in Search source. Maybe that'll already work for you.

[rs]

I'm using you lib in this project: https://github.com/rs/rest-layer-es

I already implemented timeout respecting context's deadline: https://github.com/rs/rest-layer-es/blob/master/util.go#L80

Now I'd like to respect cancellation as you mentioned in you point 1. The most obvious endpoints are Search, Get and MultiGet. Bulk would be my next, then Scroll and finally Update and Delete.

[olivere]

I implemented the basics in Client and for the Search service in the ctxhttp branch. Will you have a look?

[rs]

Looks great :)

[olivere]

Update: Proposal to add net/context to stdlib.

[olivere]

It's been a looong time, but finally, in 3.0.48 you can use context.Context will all services in Elastic (see 73e440). Simply use <Service>.DoC(context.Context) instead of <Service>.Do() to pass a context.

I will publish 3.0.48 soon. It is for Elasticsearch 2.x only.

[rs]

Awesome!

[olivere]

Yes. It will be for elastic.v3 only. I will only backport to elastic.v2 if it's really necessary. I'm unsure what to do about it in elastic.v5. Time will tell. ;-)