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 71 forks source link

[BUG] BulkParts is not convinient #93

Open GopherJ opened 4 years ago

GopherJ commented 4 years ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when I'm not able to return back BulkParts from a function.

Often we just create Index by using part of my data but the following code won't compile


fn build_bulk<'a>(
    proximity_uuid: &'a str,
    alerts: Vec<Alert>,
) -> (BulkParts<'a>, Vec<JsonBody<Value>>) {
    let bulk_parts = BulkParts::Index(&format!("alerts.{}", proximity_uuid));
    let body: Vec<JsonBody<_>> = alerts
        .into_iter()
        .map(|alert| {
            vec![
                JsonBody::from(json!({"index": {"_id": Uuid::new_v4().to_simple().to_string() }})),
                JsonBody::from(json!(alert)),
            ]
        })
        .flatten()
        .collect();

    (bulk_parts, body)
}

Describe the solution you'd like A clear and concise description of what you want to happen.

Make it possible to return back a BulkParts from a function

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

GopherJ commented 4 years ago

It'll even cause problem when we would like to bring BulkParts into async move {}, I'm not sure about the advantage over String...

russcam commented 4 years ago

thanks for opening @GopherJ. I need to have a think about this some more, but String would probably make parts easier to work with.

By the way, for the index operation, you can leave the "_id" blank if you don't care about the value assigned. Elasticsearch will generate an id for you in this scenario and indexing will be faster, because Elasticsearch no longer needs to check if there is an existing document with the provided id.

GopherJ commented 4 years ago

@russcam

By the way, for the index operation, you can leave the "_id" blank if you don't care about the value assigned. Elasticsearch will generate an id for you in this scenario and indexing will be faster, because Elasticsearch no longer needs to check if there is an existing document with the provided id.

cool I 'll adjust my code thanks for the info.

thanks for opening @GopherJ. I need to have a think about this some more, but String would probably make parts easier to work with.

yes I think maybe we need to pay attention to the usage of &'a str, if we will always write them manually I think it's ok but If we would like to create it through function it'll be hard because we will borrow a thing dropped after the function call.

But in my case finally I didn't return BulkParts from a function and I just return a String and then BulkParts::Index(&val) will compile fine.

russcam commented 4 years ago

Sorry, meant to say you can omit the "_id" field altogether :)

But in my case finally I didn't return BulkParts from a function and I just return a String and then BulkParts::Index(&val) will compile fine.

My thinking with the parts enums is that typically they would be constructed immediately next to or as part of the API call. Would like to have a think about it further