Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] BulkCreate operation should support optional _id #241

Open
sharp-pixel opened this issue Jan 29, 2024 · 2 comments
Open

[BUG] BulkCreate operation should support optional _id #241

sharp-pixel opened this issue Jan 29, 2024 · 2 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@sharp-pixel
Copy link

sharp-pixel commented Jan 29, 2024

What is the bug?

BulkCreate operation requires an id to be specified, when it is optional in the original documentation: https://www.elastic.co/guide/en/elasticsearch/reference/7.10/docs-bulk.html#bulk-api-request-body. Note that Data Streams require the use of a create operation and the _id should be auto-generated by OpenSearch as a best practice for log analytics.

How can one reproduce the bug?

Look at https://github.com/opensearch-project/opensearch-rs/blob/main/opensearch/src/root/bulk.rs#L228 where the new method requires the id.

What is the expected behavior?

new should have a variant pub fn new<S>(source: B) -> Self (or some other name as we cannot overload in Rust)

What is your host/environment?

any

Do you have any screenshots?

N/A

Do you have any additional context?

no

@sharp-pixel sharp-pixel added bug Something isn't working untriaged labels Jan 29, 2024
@Xtansia Xtansia added good first issue Good for newcomers and removed untriaged labels Jan 29, 2024
@Xtansia
Copy link
Collaborator

Xtansia commented Jan 29, 2024

Thanks for the report @sharp-pixel, would you be interested in making a PR to add this optional constructor?

@camerondurham
Copy link
Contributor

What are your thoughts on adjusting the existing API for consistency with how "optional" _id fields are handled?

This would be a breaking change, but could potentially make the API more intuitive/consistent if id was handled consistently per the API.

Instead of making a new constructor, what do you think about having a consistent handling of optional API fields?

  1. new(source: B) + id(id: S)
  2. new(id: Option<S>, source: B)

Different Handling of Optional _id Field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants