Fix test failure after lucene upgrade to 10 #3426

zane-neo · 2025-01-23T08:20:45Z

Description

After this change: opensearch-project/OpenSearch#16366, the TotalHits's value is been moved to a private field, unable to access it directly outside that class, so removing this to fix the test failure.

We need to change the .value to .value() to make sure the code runs correctly.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

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: zane-neo <zaniu@amazon.com>

peterzhuamazon · 2025-01-23T16:18:34Z

build.gradle

@@ -25,7 +25,7 @@ buildscript {
            opensearch_build += "-SNAPSHOT"
        }

-        common_utils_version = System.getProperty("common_utils.version", opensearch_build)


This should stay on opensearch_build as it will be switched to 3.0.0.0-alpha1-SNAPSHOT as the version of common-utils.

@peterzhuamazon I didn't see the 3.0.0.0-alpha1-SNAPSHOT available on maven repo: https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/common-utils/, I can't compile success from my local.

it's still failing with opensearch_build for neural as well

That is because Common Utils owner needs to update their plugins 1st to be compatible with lucene10.

peterzhuamazon · 2025-01-23T16:18:40Z

build.gradle

@@ -11,8 +11,8 @@ buildscript {
    ext {
        opensearch_group = "org.opensearch"
        isSnapshot = "true" == System.getProperty("build.snapshot", "true")
-        opensearch_version = System.getProperty("opensearch.version", "3.0.0-SNAPSHOT")
-        buildVersionQualifier = System.getProperty("build.version_qualifier", "")
+        opensearch_version = System.getProperty("opensearch.version", "3.0.0-alpha1-SNAPSHOT")


This is correct.

peterzhuamazon · 2025-01-23T16:20:07Z

plugin/build.gradle

@@ -56,7 +56,7 @@ dependencies {
    implementation group: 'org.opensearch', name: 'opensearch', version: "${opensearch_version}"
    implementation "org.opensearch.client:opensearch-rest-client:${opensearch_version}"
    // Multi-tenant SDK Client
-    implementation "org.opensearch:opensearch-remote-metadata-sdk:${opensearch_build}"
+    implementation "org.opensearch:opensearch-remote-metadata-sdk:3.0.0-SNAPSHOT"


Remote sdk should also be 3.0.0.0-alpha1-SNAPSHOT.

@dbwiddis

Same above for this: https://aws.oss.sonatype.org/content/repositories/snapshots/org/opensearch/opensearch-remote-metadata-sdk/

Remote sdk should also be 3.0.0.0-alpha1-SNAPSHOT.

We are still making changes on that for 2.19.0. I do not plan on updating to alpha until after at least code freeze date.

If anyone wants to get ahead of the game they are welcome to check out that repo and publish it locally with alpha1. All this churn as we're trying to finish up features within the week is causing a lot of wasted effort and delays.

pyek-bot

changes for lucene upgrade (TotalHits) looks good

martin-gaievski · 2025-01-23T21:06:41Z

@zane-neo why merging directly to main? I think the strategy that core took is more flexible (2 branches, main is pointing to lucene 9.12 and new branch, 3.0.0-alpha or whatever is pointing to core 3.0.0-alpha1 and lucene 10.1). Ml-commons is consumed by multiple other plugins, so to keep all version variables in gradle consistent we all need to follow same pattern in all plugins.

cc: @minalsha

dhrubo-os · 2025-01-23T21:34:03Z

@zane-neo why merging directly to main? I think the strategy that core took is more flexible (2 branches, main is pointing to lucene 9.12 and new branch, 3.0.0-alpha or whatever is pointing to core 3.0.0-alpha1 and lucene 10.1). Ml-commons is consumed by multiple other plugins, so to keep all version variables in gradle consistent we all need to follow same pattern in all plugins.

cc: @minalsha

+1

ylwu-amzn · 2025-01-23T21:38:17Z

@zane-neo why merging directly to main? I think the strategy that core took is more flexible (2 branches, main is pointing to lucene 9.12 and new branch, 3.0.0-alpha or whatever is pointing to core 3.0.0-alpha1 and lucene 10.1). Ml-commons is consumed by multiple other plugins, so to keep all version variables in gradle consistent we all need to follow same pattern in all plugins.

cc: @minalsha

Sounds reasonable, let's use same strategy of core ?

peterzhuamazon · 2025-01-23T22:47:04Z

In core, main is lucene10 and will publish snapshots in 3.0.0-alpha1-SNAPSHOT.

3.0.0-before-alpha1 is a locked branch with lucene9 and we will not build that branch anymore, the last build is to cleanup and restore 3.0.0-SNAPSHOT.

martin-gaievski · 2025-01-24T00:21:16Z

In core, main is lucene10 and will publish snapshots in 3.0.0-alpha1-SNAPSHOT.

3.0.0-before-alpha1 is a locked branch with lucene9 and we will not build that branch anymore, the last build is to cleanup and restore 3.0.0-SNAPSHOT.

got it, I understood it's the other way around. To follow core strategy we need to adopt lucene in main plugin branches, this PR is correct

dhrubo-os · 2025-01-24T00:37:31Z

In core, main is lucene10 and will publish snapshots in 3.0.0-alpha1-SNAPSHOT.
3.0.0-before-alpha1 is a locked branch with lucene9 and we will not build that branch anymore, the last build is to cleanup and restore 3.0.0-SNAPSHOT.

got it, I understood it's the other way around. To follow core strategy we need to adopt lucene in main plugin branches, this PR is correct

But we shouldn't merge this PR now. After 2.19 release, we can merge this PR.

According to @andrross

Once you update to consume 3.0.0-alpha1-SNAPSHOT then you'll have to fix the breaking changes from Lucene on your main branch. Those changes cannot be backported to 2.x. If you're busy working towards 2.19 in the short term you can postpone updating to the alpha version.

andrross · 2025-01-24T02:09:58Z

In core, main is lucene10 and will publish snapshots in 3.0.0-alpha1-SNAPSHOT.
3.0.0-before-alpha1 is a locked branch with lucene9 and we will not build that branch anymore, the last build is to cleanup and restore 3.0.0-SNAPSHOT.

got it, I understood it's the other way around. To follow core strategy we need to adopt lucene in main plugin branches, this PR is correct

But we shouldn't merge this PR now. After 2.19 release, we can merge this PR.

According to @andrross
Once you update to consume 3.0.0-alpha1-SNAPSHOT then you'll have to fix the breaking changes from Lucene on your main branch. Those changes cannot be backported to 2.x. If you're busy working towards 2.19 in the short term you can postpone updating to the alpha version.

To be clear, you don’t have to wait until after 2.19, but you do have that option if that makes things easier.

zane-neo · 2025-01-24T05:04:23Z

In core, main is lucene10 and will publish snapshots in 3.0.0-alpha1-SNAPSHOT.
3.0.0-before-alpha1 is a locked branch with lucene9 and we will not build that branch anymore, the last build is to cleanup and restore 3.0.0-SNAPSHOT.

got it, I understood it's the other way around. To follow core strategy we need to adopt lucene in main plugin branches, this PR is correct

But we shouldn't merge this PR now. After 2.19 release, we can merge this PR.
According to @andrross
Once you update to consume 3.0.0-alpha1-SNAPSHOT then you'll have to fix the breaking changes from Lucene on your main branch. Those changes cannot be backported to 2.x. If you're busy working towards 2.19 in the short term you can postpone updating to the alpha version.
To be clear, you don’t have to wait until after 2.19, but you do have that option if that makes things easier.

Yeah, we can merge this main but not backport it.

dhrubo-os · 2025-01-24T06:04:00Z

In core, main is lucene10 and will publish snapshots in 3.0.0-alpha1-SNAPSHOT.
3.0.0-before-alpha1 is a locked branch with lucene9 and we will not build that branch anymore, the last build is to cleanup and restore 3.0.0-SNAPSHOT.

got it, I understood it's the other way around. To follow core strategy we need to adopt lucene in main plugin branches, this PR is correct

But we shouldn't merge this PR now. After 2.19 release, we can merge this PR.
According to @andrross
Once you update to consume 3.0.0-alpha1-SNAPSHOT then you'll have to fix the breaking changes from Lucene on your main branch. Those changes cannot be backported to 2.x. If you're busy working towards 2.19 in the short term you can postpone updating to the alpha version.
To be clear, you don’t have to wait until after 2.19, but you do have that option if that makes things easier.
Yeah, we can merge this main but not backport it.

Agree, but let's merge this after 2.19 release as we are planning to raise few more PRs for 2.19 release.

Fix test failure after lucene upgrade to 10

7067941

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo requested review from b4sjoo, dhrubo-os, mingshl, jngz-es, model-collapse, rbhavna, ylwu-amzn, Zhangxunmt, austintlee, HenryL27 and xinyual as code owners January 23, 2025 08:20

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 08:22 — with GitHub Actions Failure

xinyual previously approved these changes Jan 23, 2025

View reviewed changes

Fix test failure after lucene upgrade to 10

686350b

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo dismissed xinyual’s stale review via 686350b January 23, 2025 08:28

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 08:29 — with GitHub Actions Failure

Fix test failure after lucene version upgraded to 10

88b0e08

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 08:59 — with GitHub Actions Failure

Change version to opensearch version alpha1

8b61241

Signed-off-by: zane-neo <zaniu@amazon.com>

zane-neo had a problem deploying to ml-commons-cicd-env January 23, 2025 10:24 — with GitHub Actions Failure

peterzhuamazon requested changes Jan 23, 2025

View reviewed changes

peterzhuamazon mentioned this pull request Jan 23, 2025

[RELEASE] Release version 3.0.0 opensearch-project/opensearch-build#3747

Open

43 tasks

pyek-bot reviewed Jan 23, 2025

View reviewed changes

pyek-bot approved these changes Jan 23, 2025

View reviewed changes

pyek-bot mentioned this pull request Jan 23, 2025

Added amazon rekognition as a trust endpoint #3419

Open

4 tasks

jngz-es approved these changes Jan 23, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix test failure after lucene upgrade to 10 #3426

Fix test failure after lucene upgrade to 10 #3426

zane-neo commented Jan 23, 2025 •

edited

Loading

peterzhuamazon Jan 23, 2025 •

edited

Loading

zane-neo Jan 24, 2025

martin-gaievski Jan 24, 2025

peterzhuamazon Jan 24, 2025 •

edited

Loading

peterzhuamazon Jan 23, 2025

peterzhuamazon Jan 23, 2025

zane-neo Jan 24, 2025

dbwiddis Jan 24, 2025

pyek-bot left a comment

martin-gaievski commented Jan 23, 2025 •

edited

Loading

dhrubo-os commented Jan 23, 2025

ylwu-amzn commented Jan 23, 2025

peterzhuamazon commented Jan 23, 2025

martin-gaievski commented Jan 24, 2025

dhrubo-os commented Jan 24, 2025

andrross commented Jan 24, 2025

zane-neo commented Jan 24, 2025

dhrubo-os commented Jan 24, 2025

Fix test failure after lucene upgrade to 10 #3426

Are you sure you want to change the base?

Fix test failure after lucene upgrade to 10 #3426

Conversation

zane-neo commented Jan 23, 2025 • edited Loading

Description

Related Issues

Check List

peterzhuamazon Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

zane-neo Jan 24, 2025

Choose a reason for hiding this comment

martin-gaievski Jan 24, 2025

Choose a reason for hiding this comment

peterzhuamazon Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

peterzhuamazon Jan 23, 2025

Choose a reason for hiding this comment

peterzhuamazon Jan 23, 2025

Choose a reason for hiding this comment

zane-neo Jan 24, 2025

Choose a reason for hiding this comment

dbwiddis Jan 24, 2025

Choose a reason for hiding this comment

pyek-bot left a comment

Choose a reason for hiding this comment

martin-gaievski commented Jan 23, 2025 • edited Loading

dhrubo-os commented Jan 23, 2025

ylwu-amzn commented Jan 23, 2025

peterzhuamazon commented Jan 23, 2025

martin-gaievski commented Jan 24, 2025

dhrubo-os commented Jan 24, 2025

andrross commented Jan 24, 2025

zane-neo commented Jan 24, 2025

dhrubo-os commented Jan 24, 2025

zane-neo commented Jan 23, 2025 •

edited

Loading

peterzhuamazon Jan 23, 2025 •

edited

Loading

peterzhuamazon Jan 24, 2025 •

edited

Loading

martin-gaievski commented Jan 23, 2025 •

edited

Loading