elastic / elasticsearch-rs

Official Elasticsearch Rust Client
https://www.elastic.co/guide/en/elasticsearch/client/rust-api/current/index.html
Apache License 2.0
695 stars 70 forks source link

[BUG] IndicesPutTemplate has an unnecessary bound B #153

Closed GopherJ closed 3 years ago

GopherJ commented 3 years ago

Describe the bug A clear and concise description of what the bug is.

https://docs.rs/elasticsearch/7.9.0-alpha.1/src/elasticsearch/generated/namespace_clients/indices.rs.html#6203-6205 has a bound B but it's not necessary and redundant to T in https://docs.rs/elasticsearch/7.9.0-alpha.1/src/elasticsearch/generated/namespace_clients/indices.rs.html#6230

To Reproduce Steps to reproduce the behavior:

Expected behavior A clear and concise description of what you expected to happen.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

Additional context Add any other context about the problem here.

GopherJ commented 3 years ago

I believe that we will only need one of them

russcam commented 3 years ago

Hi @GopherJ,

T and B serve different purposes; T is a type that implements Serialize, whilst B is a type that implements Body.

client.indices().put_template() returns a new instance of IndicesPutTemplate<'a, 'b, ()> where B is (). Then when body() is later called, it returns a new instance of IndicesPutTemplate where B is JsonBody<T>.

If the body call were constrained to B, then a user would need to wrap any type that implements Serialize in JsonBody e.g. JsonBody(json!({ })), which feels more cumbersome to use that simply passing the type. Accepting B would however have the benefit of accepting other Body implementations like String, &[u8], etc.

In the future, T may end up being a specific type for IndicesPutTemplate (#75), though it's not definite how this will be implemented; your thoughts are welcome on #75.

My inclination is to leave this as is for now.

russcam commented 3 years ago

Closing this as I think I have explained how T and B are used in the current implementation