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

Fixes #283 #538

Merged
merged 2 commits into from
Jul 3, 2023
Merged

Fixes #283 #538

merged 2 commits into from
Jul 3, 2023

Conversation

urinud
Copy link
Contributor

@urinud urinud commented Jun 22, 2023

Description

Adds the option to set slices=auto for UpdateByQueryRequest, DeleteByQueryRequest and ReindexRequest.
Using value 0 (zero) to express auto slicing.
This maintains consistency with elastic/elasticsearch#53068

Issues Resolved

Fixes #283

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@reta
Copy link
Collaborator

reta commented Jun 26, 2023

Thanks for the pull request @urinud , could you please add a few test cases to verify the change under AbstractRequestIT (or other appropriate test suite)? thank you.

@dblock
Copy link
Member

dblock commented Jun 26, 2023

Also this is a breaking change to existing users, isn't it?

@urinud
Copy link
Contributor Author

urinud commented Jun 26, 2023

Also this is a breaking change to existing users, isn't it?

I don’t think is a breaking change as zero is not a valid value for the slices. Also this change maintains consistency with the equivalent feature in Elastic.

Copy link
Collaborator

@VachaShah VachaShah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@urinud Can you add a changelog entry in CHANGELOG for this PR?

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Please remove ES license from new files, and confirm that you're not copying any code from non-open-source (APLv2-compatible) codebases.

For DCO I think you need to amend your commits. LMK if you need help with that?

@urinud urinud force-pushed the Fix-283 branch 3 times, most recently from 0eeae4a to a477810 Compare June 28, 2023 22:30
@urinud urinud requested review from dblock and VachaShah June 28, 2023 22:34
CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Uriel Dan Nudelman <urinud@gmail.com>
Signed-off-by: Uriel Dan Nudelman <urinud@gmail.com>
@urinud urinud requested a review from dblock June 28, 2023 23:33
@urinud
Copy link
Contributor Author

urinud commented Jul 3, 2023

Hello @reta and @VachaShah can you please help reviewing the PR? Is there anything else I should have added?
Thank you

@reta reta merged commit 8525530 into opensearch-project:main Jul 3, 2023
30 checks passed
@reta reta added the backport 2.x Backport to 2.x branch label Jul 3, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 3, 2023
Signed-off-by: Uriel Dan Nudelman <urinud@gmail.com>
(cherry picked from commit 8525530)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta pushed a commit that referenced this pull request Jul 3, 2023
(cherry picked from commit 8525530)

Signed-off-by: Uriel Dan Nudelman <urinud@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@BrendonFaleiro BrendonFaleiro mentioned this pull request Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Auto slicing is not supported for DeleteByQuery, Reindex, and UpdateByQuery
4 participants