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

Update bwc version after 2.9 release #1002

Merged
merged 2 commits into from
Jul 26, 2023

Conversation

junqiu-lei
Copy link
Member

@junqiu-lei junqiu-lei commented Jul 24, 2023

Description

Update versions after 2.9 release: change 2.9.0-SNAPSHOT to 2.9.0, add 2.10.0-SNAPSHOT.

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed as per the DCO using --signoff

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: Junqiu Lei <junqiu@amazon.com>
@junqiu-lei junqiu-lei added Maintenance Add support for new versions of OpenSearch/Dashboards from upstream skip-changelog labels Jul 24, 2023
vamshin
vamshin previously approved these changes Jul 25, 2023
Copy link
Member

@vamshin vamshin left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@navneet1v
Copy link
Collaborator

@junqiu-lei not sure if this change fixes the BWC tests, as I can see the tests in the github actions are failing.

@junqiu-lei
Copy link
Member Author

@navneet1v The BWC tests error is unrelated to this PR:

> Task :generateTestFixturesEffectiveLombokConfig
/home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/ODFERestTestCase.java:188: error: cannot find symbol

        XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType());
                                                ^
  symbol:   method fromMediaType(String)
  location: class XContentType
> Task :compileTestFixturesJava
/home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java:1277: error: cannot find symbol

        XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType());
> Task :compileTestFixturesJava FAILED
                                                ^
  symbol:   method fromMediaType(String)
  location: class XContentType
Note: /home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/ODFERestTestCase.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: /home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
2 errors

The error is same as the Geospatial plugin one fixed in the PR. I can help to raise a PR later to fix it.

@navneet1v
Copy link
Collaborator

@navneet1v The BWC tests error is unrelated to this PR:

Block (21 lines)

> Task :generateTestFixturesEffectiveLombokConfig
/home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/ODFERestTestCase.java:188: error: cannot find symbol

        XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType());
                                                ^
  symbol:   method fromMediaType(String)
  location: class XContentType
> Task :compileTestFixturesJava
/home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java:1277: error: cannot find symbol

        XContentType xContentType = XContentType.fromMediaType(response.getEntity().getContentType());
> Task :compileTestFixturesJava FAILED
                                                ^
  symbol:   method fromMediaType(String)
  location: class XContentType
Note: /home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/ODFERestTestCase.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: /home/runner/work/k-NN/k-NN/src/testFixtures/java/org/opensearch/knn/KNNRestTestCase.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
2 errors

The error is same as the Geospatial plugin one fixed in the PR. I can help to raise a PR later to fix it.

can we merge in those changes, as this is not very difficult to know why the tests are not passing.

@codecov
Copy link

codecov bot commented Jul 26, 2023

Codecov Report

Merging #1002 (f586df1) into main (5b26be3) will decrease coverage by 0.24%.
The diff coverage is 100.00%.

❗ Current head f586df1 differs from pull request most recent head 01c4e91. Consider uploading reports for the commit 01c4e91 to get more accurate results

@@             Coverage Diff              @@
##               main    #1002      +/-   ##
============================================
- Coverage     85.37%   85.13%   -0.24%     
+ Complexity     1146     1141       -5     
============================================
  Files           154      154              
  Lines          4683     4683              
  Branches        423      423              
============================================
- Hits           3998     3987      -11     
- Misses          495      504       +9     
- Partials        190      192       +2     
Files Changed Coverage Δ
...index/codec/KNN80Codec/KNN80DocValuesConsumer.java 88.69% <100.00%> (ø)

... and 2 files with indirect coverage changes

@navneet1v
Copy link
Collaborator

@junqiu-lei I can still see BWC tests failing. Please check

@junqiu-lei
Copy link
Member Author

There is a new error on the BWC CI:

| Output for ./bin/opensearch-plugin:-> Installing file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
> Task :qa:restart-upgrade:testAgainstOldCluster FAILED
| -> Downloading file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Failed installing file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Rolling back file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Rolled back file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| Exception in thread "main" java.io.FileNotFoundException: /home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip (No such file or directory)
| 	at java.base/java.io.FileInputStream.open0(Native Method)
| 	at java.base/java.io.FileInputStream.open(FileInputStream.java:216)
| 	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157)

@naveentatikonda
Copy link
Member

There is a new error on the BWC CI:

| Output for ./bin/opensearch-plugin:-> Installing file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
> Task :qa:restart-upgrade:testAgainstOldCluster FAILED
| -> Downloading file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Failed installing file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Rolling back file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Rolled back file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| Exception in thread "main" java.io.FileNotFoundException: /home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip (No such file or directory)
| 	at java.base/java.io.FileInputStream.open0(Native Method)
| 	at java.base/java.io.FileInputStream.open(FileInputStream.java:216)
| 	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157)

It is failing because k-NN plugin might not be present in the new artifact, 2.10

@naveentatikonda
Copy link
Member

There is a new error on the BWC CI:

| Output for ./bin/opensearch-plugin:-> Installing file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
> Task :qa:restart-upgrade:testAgainstOldCluster FAILED
| -> Downloading file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Failed installing file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Rolling back file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| -> Rolled back file:/home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip
| Exception in thread "main" java.io.FileNotFoundException: /home/runner/work/k-NN/k-NN/qa/restart-upgrade/build/private/artifact_tmp/opensearch-knn-2.10.0.zip (No such file or directory)
| 	at java.base/java.io.FileInputStream.open0(Native Method)
| 	at java.base/java.io.FileInputStream.open(FileInputStream.java:216)
| 	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:157)

@junqiu-lei plugins are not added in the manifest 2.10.0. So, the artifact don't have the plugins and the BWC tests are failing

@naveentatikonda
Copy link
Member

Please fix the DCO check

@junqiu-lei
Copy link
Member Author

Looks like infra team will help to add all components to manifest. cc @prudhvigodithi opensearch-project/opensearch-build@8fb3588

And also we need merge the 2.x PR BWC bump: #979

@naveentatikonda
Copy link
Member

@junqiu-lei Can you also pls fix the Developer Certificate of Origin Check. I think if we don't fix it now, it will cause the DCO check on future PRs also to fail.

Signed-off-by: Junqiu Lei <junqiu@amazon.com>
@junqiu-lei
Copy link
Member Author

Thanks @naveentatikonda, added DCO

@junqiu-lei
Copy link
Member Author

junqiu-lei commented Jul 26, 2023

The CI failure is expected to fail as k-NN component still not added to manifest 2.10.0.

@junqiu-lei junqiu-lei merged commit 8c79385 into opensearch-project:main Jul 26, 2023
9 of 37 checks passed
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1002-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 8c79385330d80d225460a713effbaa039aaa886a
# Push it to GitHub
git push --set-upstream origin backport/backport-1002-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1002-to-2.x.

@junqiu-lei
Copy link
Member Author

Will manually raise PR to fix the core refactor to 2.x and then merge #979

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Maintenance Add support for new versions of OpenSearch/Dashboards from upstream skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants