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

[ENHANCEMENT] Implement From<T> for UrlParts enums

Open russcam opened this issue 4 years ago • 3 comments

Each API models the API url parts as an enum, and where an API has more than one enum variant, the API function on the root/namespace client takes the enum as an argument. For example, for search

let response = client
    .search(SearchUrlParts::Index(&["index-1"])) // <-- accepts enum
    .send()
    .await?;

Currently, the Rust compiler cannot infer the enum type based on the parameter, meaning the complete enum path needs to be specified as above, instead of simply the following pseudo

let response = client
    .search(Index(&["index-1"]))
    .send()
    .await?;

To make the API somewhat simpler to use, it is proposed to implement From<T> traits for each enum such that a value, or tuple of values, can be used. Taking the example above

impl<'a> From<&'a [&'a str]> for SearchUrlParts<'a> {
    fn from(index: &'a [&'a str]) -> Self {
        SearchUrlParts::Index(index)
    }
}

impl<'a> From<(&'a [&'a str], &'a [&'a str])> for SearchUrlParts<'a> {
    fn from(index_type: (&'a [&'a str], &'a [&'a str])) -> Self {
        SearchUrlParts::IndexType(index_type.0, index_type.1)
    }
}

let response = client
    .search(&["posts"].into())
    .send()
    .await?;

russcam avatar Nov 29 '19 01:11 russcam

I was thinking the exact same thing when I was reading the docs.

Should we go a step further and have search accept an Into<SearchUrlParts<'a>>?

Also, what should we do when the variants are the same type?

mwilliammyers avatar Jan 09 '20 07:01 mwilliammyers

Should we go a step further and have search accept an Into<SearchUrlParts<'a>>?

👍 to this

Also, what should we do when the variants are the same type?

As far as I am aware, I think there are only a few APIs where the enum variant to construct from a passed tuple of values is ambiguous

  • NodesInfoParts: https://github.com/elastic/elasticsearch-rs/blob/76d5c2ea3b59f2a9224b4f5344aa2fe39844b0a0/elasticsearch/src/generated/namespace_clients/nodes.rs#L214-L223
  • NodesStatsParts: https://github.com/elastic/elasticsearch-rs/blob/76d5c2ea3b59f2a9224b4f5344aa2fe39844b0a0/elasticsearch/src/generated/namespace_clients/nodes.rs#L527-L540
  • NodesUsageParts: https://github.com/elastic/elasticsearch-rs/blob/76d5c2ea3b59f2a9224b4f5344aa2fe39844b0a0/elasticsearch/src/generated/namespace_clients/nodes.rs#L784-L793

In these cases, I would propose choosing the more common variant?

russcam avatar Jan 22 '20 04:01 russcam

Started looking at this in enhancement/24 branch.

russcam avatar Jul 16 '20 01:07 russcam