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

Fixing integ tests due to missing opensearch-job-scheduler plugin and jayway lib #1147

Merged

Conversation

yizheliu-amazon
Copy link
Contributor

@yizheliu-amazon yizheliu-amazon commented Jan 29, 2025

Description

Fixing integ tests due to missing opensearch-job-scheduler plugin.

This is needed since ML commons introduces new dependency on job-scheduler: https://github.com/opensearch-project/ml-commons/pull/3421/files#diff-520f178549fabd7e74f5b1fedb7275a1434a2ac153fc49e0212455ceeef38e37R47

Before this change, integ test will fail due to below job-scheduler missing error:

| Exception in thread "main" java.lang.IllegalArgumentException: Missing plugin [opensearch-job-scheduler], dependency of [opensearch-ml]
|       at org.opensearch.plugins.PluginsService.addSortedBundle(PluginsService.java:527)
|       at org.opensearch.plugins.PluginsService.sortBundles(PluginsService.java:495)
|       at org.opensearch.plugins.InstallPluginCommand.jarHellCheck(InstallPluginCommand.java:833)
|       at org.opensearch.plugins.InstallPluginCommand.loadPluginInfo(InstallPluginCommand.java:807)
|       at org.opensearch.plugins.InstallPluginCommand.installPlugin(InstallPluginCommand.java:852)
|       at org.opensearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:274)
|       at org.opensearch.plugins.InstallPluginCommand.execute(InstallPluginCommand.java:248)
|       at org.opensearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:104)
|       at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
|       at org.opensearch.cli.MultiCommand.execute(MultiCommand.java:104)
|       at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
|       at org.opensearch.cli.Command.main(Command.java:101)
|       at org.opensearch.plugins.PluginCli.main(PluginCli.java:66)

FAILURE: Build failed with an exception.

Also, integ test fail with the error saying below, which is captured in #1145

  2> REPRODUCE WITH: ./gradlew ':integTest' --tests "org.opensearch.neuralsearch.processor.SparseEncodingProcessIT.testSparseEncodingProcessor" -Dtests.seed=C9C47530B30641AB -Dtests.security.manager=false -Dtests.locale=cs -Dtests.timezone=America/Fort_Nelson -Druntime.java=21
  2> org.opensearch.client.ResponseException: method [POST], host [http://[::1]:41111], URI [sparse_encoding_index/_doc?refresh], status line [HTTP/1.1 500 Internal Server Error]
    {"error":{"root_cause":[{"type":"null_pointer_exception","reason":"Cannot invoke \"org.opensearch.ml.common.output.model.ModelTensorOutput.getMlModelOutputs()\" because \"modelTensorOutput\" is null"}],"type":"null_pointer_exception","reason":"Cannot invoke \"org.opensearch.ml.common.output.model.ModelTensorOutput.getMlModelOutputs()\" because \"modelTensorOutput\" is null"},"status":500}

Root cause of this error is that jayway lib is not included as runtime dependency

»       at java.base/java.lang.Thread.run(Thread.java:1583) [?:?]
»  Caused by: java.lang.NoClassDefFoundError: com/jayway/jsonpath/PathNotFoundException
»       at org.opensearch.ml.common.output.model.ModelTensor.<init>(ModelTensor.java:235) ~[opensearch-ml-common-2.19.0.0.jar:?]
»       at org.opensearch.ml.common.output.model.ModelTensors.<init>(ModelTensors.java:66) ~[opensearch-ml-common-2.19.0.0.jar:?]
»       at org.opensearch.ml.common.output.model.ModelTensorOutput.<init>(ModelTensorOutput.java:44) ~[opensearch-ml-common-2.19.0.0.jar:?]
»       at java.base/jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) ~[?:?]
»       ... 21 more
»  Caused by: java.lang.ClassNotFoundException: com.jayway.jsonpath.PathNotFoundException

Related Issues

#1145
ML-commons side: opensearch-project/ml-commons#3464

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.

@q-andy
Copy link

q-andy commented Jan 29, 2025

Does this resolve #1145?

Copy link
Member

@owaiskazi19 owaiskazi19 left a comment

Choose a reason for hiding this comment

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

LGTM with a minor comment

@weijia-aws
Copy link

Does this resolve #1145?

No, the tests are still failing. In fact, I don't think this commit will fix the integ tests, instead it fixes the build failure that caused by:

Exception in thread "main" java.lang.IllegalArgumentException: Missing plugin [opensearch-job-scheduler], dependency of [opensearch-ml]

Which happens before running the integ tests

junqiu-lei
junqiu-lei previously approved these changes Jan 29, 2025
@junqiu-lei junqiu-lei added backport 2.x Label will add auto workflow to backport PR to 2.x branch skip-changelog labels Jan 29, 2025
Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
@vibrantvarun
Copy link
Member

Please wait for all CI checks to pass

@martin-gaievski
Copy link
Member

is this related to some changes in ml-commons?

NeuralQueryEnricherProcessorIT > testNeuralQueryEnricherProcessor_NeuralSparseSearch_E2EFlow FAILED
    org.opensearch.client.ResponseException: method [POST], host [http://[::1]:44529], URI [/neuralsearch-bwc-testneuralqueryenricherprocessor_neuralsparsesearch_e2eflow/_doc/0?refresh=true], status line [HTTP/1.1 500 Internal Server Error]
    {"error":{"root_cause":[{"type":"null_pointer_exception","reason":"Cannot invoke \"org.opensearch.ml.common.output.model.ModelTensorOutput.getMlModelOutputs()\" because \"modelTensorOutput\" is null"}],"type":"null_pointer_exception","reason":"Cannot invoke \"org.opensearch.ml.common.output.model.ModelTensorOutput.getMlModelOutputs()\" because \"modelTensorOutput\" is null"},

@vibrantvarun
Copy link
Member

This issue is coming from ML commons

@vibrantvarun
Copy link
Member

Do not merge this PR for now

@yizheliu-amazon
Copy link
Contributor Author

Same issue as identified in #1145

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
@yizheliu-amazon yizheliu-amazon changed the title Fixing integ tests due to missing opensearch-job-scheduler plugin Fixing integ tests due to missing opensearch-job-scheduler plugin and jayway lib Jan 30, 2025
@vibrantvarun
Copy link
Member

@yizheliu-amazon attach the relevant issues in the PR description. The bug opened by @WeijiaZhao on neural

@vibrantvarun
Copy link
Member

@yizheliu-amazon I think we need to add this in qa/build.gradle as well

…test

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
@yizheliu-amazon
Copy link
Contributor Author

Added same change to qa/build.gradle, but still not working. Reproducing on my local now.

@vibrantvarun
Copy link
Member

Hi @yizheliu-amazon here you need to do following things

  1. Provide following line in qa/build.gradle
   // json-path 2.9.0 depends on slf4j 2.0.11, which conflicts with the version used by OpenSearch core.
    // Excluding slf4j here since json-path is only used for testing, and logging failures in this context are acceptable.
   // OpenSearch core is using slf4j 1.7.36. Therefore, we cannot change the version here.
    testRuntimeOnly('com.jayway.jsonpath:json-path:2.9.0') {
        exclude group: 'org.slf4j', module: 'slf4j-api'
    }
  1. Provide following line in build.gradle
   // json-path 2.9.0 depends on slf4j 2.0.11, which conflicts with the version used by OpenSearch core.
    // Excluding slf4j here since json-path is only used for testing, and logging failures in this context are acceptable.
   // OpenSearch core is using slf4j 1.7.36. Therefore, we cannot change the version here.
    runtimeOnly('com.jayway.jsonpath:json-path:2.9.0') {
        exclude group: 'org.slf4j', module: 'slf4j-api'
    }
  1. BWC tests of 2.19.0-SNAPSHOT with 3.0.0 will fail because this change is not present in 2.19 branch. Once we merge this PR it will backported to that branch. POC: Test Pr 2 #1149

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
@yizheliu-amazon
Copy link
Contributor Author

Incorporated changes of PR #1148 from @vibrantvarun to this PR, so that Sl4j is excluded, and jayway lib can be used correctly.

@vibrantvarun
Copy link
Member

Note: BWC tests with branch 2.19.0-SNAPSHOT with 3.0 will continue to fail until these change is backported to 2.x

@vibrantvarun
Copy link
Member

Also BWC tests with 2.18.0 with 3.0.0 are passing because older version is picking dependencies in right fashion. Once this PR goes to 2.x then 2.19 will also be start passing.

@vibrantvarun vibrantvarun dismissed junqiu-lei’s stale review January 30, 2025 18:16

Need fresh review as it contains some new changes.

@vibrantvarun
Copy link
Member

@junqiu-lei / @martin-gaievski Can one you review this PR now? From my side it is LGTM and green

Copy link
Member

@junqiu-lei junqiu-lei left a comment

Choose a reason for hiding this comment

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

@yizheliu-amazon Thanks for the fix, LGTM.

@vibrantvarun vibrantvarun merged commit 558da1f into opensearch-project:main Jan 30, 2025
21 of 41 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 30, 2025
… jayway lib (#1147)

* Fixing integ tests due to missing opensearch-job-scheduler plugin

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to fix issue #1145

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to qa/build.gradle to fix issue bwc test

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Exclude Sl4j to make sure jayway lib is being used

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

---------

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
(cherry picked from commit 558da1f)
@yizheliu-amazon yizheliu-amazon deleted the add-job-scheduler branch January 30, 2025 18:24
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 30, 2025
… jayway lib (#1147)

* Fixing integ tests due to missing opensearch-job-scheduler plugin

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to fix issue #1145

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to qa/build.gradle to fix issue bwc test

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Exclude Sl4j to make sure jayway lib is being used

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

---------

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
(cherry picked from commit 558da1f)
vibrantvarun pushed a commit that referenced this pull request Jan 30, 2025
… jayway lib (#1147) (#1150)

* Fixing integ tests due to missing opensearch-job-scheduler plugin

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to fix issue #1145

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to qa/build.gradle to fix issue bwc test

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Exclude Sl4j to make sure jayway lib is being used

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

---------

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
(cherry picked from commit 558da1f)

Co-authored-by: Yizhe Liu <59710443+yizheliu-amazon@users.noreply.github.com>
martin-gaievski pushed a commit that referenced this pull request Jan 30, 2025
… jayway lib (#1147) (#1151)

* Fixing integ tests due to missing opensearch-job-scheduler plugin

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to fix issue #1145

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Add jayway as runtime dependency to qa/build.gradle to fix issue bwc test

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

* Exclude Sl4j to make sure jayway lib is being used

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>

---------

Signed-off-by: Yizhe Liu <yizheliu@amazon.com>
(cherry picked from commit 558da1f)

Co-authored-by: Yizhe Liu <59710443+yizheliu-amazon@users.noreply.github.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 backport 2.19 skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants