Rust: Prototype of generic HttpFgbReader #345
Conversation
|
Hi @kylebarron - is there a reason that this is opened as a draft? What further changes are you intending to make? |
Mostly just that it was a proposal and I hadn't opened an issue previously to discuss it, and I wasn't sure if people were interested in such a change. |
There was a problem hiding this comment.
The goal seems reasonable and the changes LGTM.
As well as this flatgeobuf crate, @pka also maintains the http-range-client crate, so it'd be good to get a review from him as well.
There was a problem hiding this comment.
I'm all for supporting other HTTP clients.
My initial idea was to include other clients in the http-range-client crate and build with approprate feature flags. But having the possibilty to use your own client implementation with flatgeobuf would justify having a generic HttpFgbReader.
| @@ -13,7 +13,7 @@ keywords = ["geo", "r-tree", "spatial"] | |||
|
|
|||
| [features] | |||
| default = ["http"] | |||
| http = ["http-range-client", "bytes"] | |||
| http = ["http-range-client", "bytes", "reqwest"] | |||
There was a problem hiding this comment.
I'm puzzled that reqwest becomes a strict dependency, when the goal of using the generic AsyncBufferedHttpRangeClient is to support other clients?
There was a problem hiding this comment.
I think this was to keep backwards compatibility. Reqwest is already a dependency via http-range-client, but now we need to import the reqwest::Client type to keep HttpFgbReader::open the same. https://github.com/flatgeobuf/flatgeobuf/pull/345/files#diff-f6dfac8fdb80ad62f8c61b19711f73cda3c26c1467a5c6bf1cb891e57b582311R37
|
Can I do something to help move this PR forward? Anything blocking merge? |
This is a draft for now because `HTTPFgbReader` is not generic over backends. See flatgeobuf/flatgeobuf#345
Right now the Flatgeobuf crates has a synchronous
FgbReaderand an asyncHttpFgbReader. But theHttpFgbReaderis not generic. It requires an underlyingreqwest::Client. It would be great to allow this underlying backend to be generic.The motivation for this is to connect Flatgeobuf to the object-store crate, which abstracts among various object storage providers including S3, GCS, Azure, etc. In geoarrow/geoarrow-rs#494 I have an example of connecting object-store to
http_range_client::AsyncHttpRangeClient(https://github.com/geoarrow/geoarrow-rs/pull/494/files#diff-4bc105a5ea08ab4e7e6d0bea2453dcd5e3a7c89d531019042abfeeca30b4b83c), but I can't construct aFlatGeobuf::HttpFgbReaderthat's generic over that backend.