search

benashford / rs-es

A Rust client for the ElasticSearch REST API
Apache License 2.0
217 stars 44 forks source link

Upgrade Hyper to 0.11 #110

Closed benashford closed 5 years ago

benashford commented 7 years ago

Hyper 0.11 introduces asynchronous processing with Tokio and Futures: https://hyper.rs/guides/client/basic/

We should consider how to modify rs-es to use it. In principal we should adopt Futures as well, this is because ElasticSearch operations can involve non-trivial waiting time, and this would allow application authors to use ElasticSearch without blocking threads, etc.

alexkornitzer commented 6 years ago

Hey, could you bump to this so that rs-es supports openssl 1.1.1? Or does that introduce major breaking changes?

benashford commented 6 years ago

It will introduce breaking changes, as everything will become asynchronous afterwards, so each operation that returns a Result<T, EsError> will return a impl Future<Item = T, Err = EsError instead. And users of this library would need to change to run in a Tokio runtime instead.

But given that Hyper has gone down that direction, and the whole Rust world is moving in that direction, it's a breaking change I think we should make.

The only reason not to is that the futures based world is still moving quite fast. There would need to be a breaking change again when Futures 0.3 is released, for example.

alexkornitzer commented 6 years ago

Ah I see that makes total sense. So when more distros start to push openssl 1.1.1, the only solution will be to use futures? As by looking at it there is no easy way to just bump the openssl cargo.

benashford commented 6 years ago

It looks that way, looking at the later releases of hyper. The default path would be to use hypertls rather than hyper-openssl, which in turn uses native-tls which will use the latest openssl on Linux platforms. But hypertls was only introduced after Hyper 0.11, the first release to be Futures based.

To support newer versions of openssl with Hyper 0.10 we'd need a different connector that supported that version of Hyper. But I'm not aware of any.

alexkornitzer commented 5 years ago

Back on the topic of this, I was just wondering if it would be worth abstracting one level further and just building rs-es on top of reqwest? This would be an easier API to keep in line with and most elastic projects need the reqwest library anyway to cover the aspects of elastic that this library does not e.g. create/delete indexes... Or would this bloat the library more than you would like?

Was just a thought as for me the most important part of this library is the query builder.

benashford commented 5 years ago

Closing this issue as #126 means we're using Hyper 0.12 indirectly via Reqwest.