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

Remove batch_size of bulk API from tests & refactor BWC version check #852

Merged
merged 7 commits into from
Aug 23, 2024

Conversation

chishui
Copy link
Contributor

@chishui chishui commented Jul 29, 2024

Description

  1. Remove the use of batch_size of bulk API from tests
  2. Update BWC tests to use batch_size in processor
  3. Refactor BWC version check in gradle file

Related Issues

Resolves opensearch-project/OpenSearch#14283

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • [] Public documentation issue/PR created.

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.

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
Copy link
Member

@zhichao-aws zhichao-aws left a comment

Choose a reason for hiding this comment

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

Thanks for the refactor on the versionsBelow check logic.

@@ -124,13 +124,25 @@ protected String uploadSparseEncodingModel() throws Exception {
return registerModelGroupAndGetModelId(requestBody);
}

protected void createPipelineForSparseEncodingProcessor(String modelId, String pipelineName) throws Exception {
protected void createPipelineForSparseEncodingProcessor(String modelId, String pipelineName, Integer batchSize) throws Exception {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like repeating code, is there any opportunity to optimize this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Too many functions are duplicate between these two bwc files, I think we need dedicated work to refactor

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
@zane-neo zane-neo merged commit e1c3878 into opensearch-project:main Aug 23, 2024
34 of 35 checks passed
@zane-neo zane-neo added the backport 2.x Label will add auto workflow to backport PR to 2.x branch label Aug 23, 2024
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 23, 2024
…#852)

* Remove batch_size of bulk API from tests & refactor BWC version check

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Update changelog

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address some comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Update Changelog

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

---------

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
(cherry picked from commit e1c3878)
martin-gaievski pushed a commit that referenced this pull request Oct 23, 2024
…#852)

* Remove batch_size of bulk API from tests & refactor BWC version check

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Update changelog

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Address some comments

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

* Update Changelog

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>

---------

Signed-off-by: Liyun Xiu <xiliyun@amazon.com>
(cherry picked from commit e1c3878)
Signed-off-by: Martin Gaievski <gaievski@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Label will add auto workflow to backport PR to 2.x branch skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Make batch ingestion automatic, not a parameter on _bulk
3 participants