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

[ENHANCEMENT] index and create method argument from BulkOperation #154

Closed Stargateur closed 3 years ago

Stargateur commented 3 years ago

index is not logic, if I understand correctly, the purpose of index is "create or replace", so why it doesn't take a id argument like create, that is almost equivalent.

Possible fix:

russcam commented 3 years ago

Hi @Stargateur, the bulk operations model the bulk API:

¹ this has recently changed with the introduction of data streams. A version of the client can be used with any version of Elasticsearch with the same major version though, so we probably still want to enforce an _id to be passed, because there are Elasticsearch 7.x versions where the _id is required for a create operation.

Stargateur commented 3 years ago

the doc clearly state in create:

_id (Optional, string) The document ID. If no ID is specified, a document ID is automatically generated.

it's only required by delete so I maintain what I said

¹ this has recently changed with the introduction of data streams. A version of the client can be used with any version of Elasticsearch with the same major version though, so we probably still want to enforce an _id to be passed, because there are Elasticsearch 7.x versions where the _id is required for a create operation.

I give up, elk have too much random change in major version I can't follow.

I mean, my first proposition is way better for the future, a good API is way better than follow a behavior in the 7.0 version of elk not documentation, you could still generate the id if the user provide none, but do what you want. At least index should take a Into<Option<String>> because it's clearly better