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

[Backport 2.x] Fix MLModelTool returns null if the response of LLM is a pure json object #2675

Merged
merged 2 commits into from
Jul 19, 2024

Conversation

qianheng-aws
Copy link
Contributor

Description

Backport 007b914 from #2655

And fix compiling failure by removing java21 API. (main use java 21 while 2.x use java17)

Issues Resolved

#2654

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed 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.

qianheng-aws and others added 2 commits July 18, 2024 20:45
…ject (opensearch-project#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)
@qianheng-aws qianheng-aws changed the title [Backport 2.x] Fix MLModelTool returns null if the response of LLM is a pure json object #2674 [Backport 2.x] Fix MLModelTool returns null if the response of LLM is a pure json object Jul 18, 2024
@dhrubo-os
Copy link
Collaborator

dhrubo-os commented Jul 18, 2024

Let's not diverge the code. Let's apply this into main too. I meant:

               Map<String, ?> dataAsMap = mlModelOutputs.get(0).getMlModelTensors().get(0).getDataAsMap();
             

@qianheng-aws
Copy link
Contributor Author

Let's not diverge the code. Let's apply this into main too. I meant:

               Map<String, ?> dataAsMap = mlModelOutputs.get(0).getMlModelTensors().get(0).getDataAsMap();
             

Raised this PR #2682 to avoid diverge on main branch.

@b4sjoo
Copy link
Collaborator

b4sjoo commented Jul 19, 2024

@dhrubo-os What about backporting this PR change into 2.13, 2.15, and better 2.14 (although it does not have patch release but we should keep code base as consistent as possible).

@dhrubo-os
Copy link
Collaborator

@dhrubo-os What about backporting this PR change into 2.13, 2.15, and better 2.14 (although it does not have patch release but we should keep code base as consistent as possible).

Yup we should do that.

@dhrubo-os dhrubo-os merged commit 0a6a2b0 into opensearch-project:2.x Jul 19, 2024
17 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 19, 2024
… a pure json object (#2675)

* Fix MLModelTool returns null if the response of LLM is a pure json object (#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)

* remove java21 API for backporting to 2.x

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 0a6a2b0)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 19, 2024
… a pure json object (#2675)

* Fix MLModelTool returns null if the response of LLM is a pure json object (#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)

* remove java21 API for backporting to 2.x

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 0a6a2b0)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jul 19, 2024
… a pure json object (#2675)

* Fix MLModelTool returns null if the response of LLM is a pure json object (#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)

* remove java21 API for backporting to 2.x

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 0a6a2b0)
b4sjoo pushed a commit that referenced this pull request Jul 19, 2024
… a pure json object (#2675) (#2685)

* Fix MLModelTool returns null if the response of LLM is a pure json object (#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)

* remove java21 API for backporting to 2.x

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 0a6a2b0)

Co-authored-by: qianheng <[email protected]>
@b4sjoo
Copy link
Collaborator

b4sjoo commented Jul 19, 2024

@dhrubo-os Remember me if I was wrong, did we come into a conclusion that if we need to backport this PR to 2.11 or not?

@dhrubo-os
Copy link
Collaborator

@dhrubo-os Remember me if I was wrong, did we come into a conclusion that if we need to backport this PR to 2.11 or not?

We introduced Agent feature from 2.13: https://opensearch.org/docs/latest/ml-commons-plugin/agents-tools/index/

b4sjoo pushed a commit that referenced this pull request Jul 20, 2024
… a pure json object (#2675) (#2683)

* Fix MLModelTool returns null if the response of LLM is a pure json object (#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)

* remove java21 API for backporting to 2.x

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 0a6a2b0)

Co-authored-by: qianheng <[email protected]>
b4sjoo pushed a commit that referenced this pull request Jul 20, 2024
… a pure json object (#2675) (#2684)

* Fix MLModelTool returns null if the response of LLM is a pure json object (#2655)

* Fix MLModelTool returns null if the response of LLM is a pure json object

Signed-off-by: Heng Qian <[email protected]>

* Fix UT failure

Signed-off-by: Heng Qian <[email protected]>

* Avoid NPE

Signed-off-by: Heng Qian <[email protected]>

* spotlessApply

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 007b914)

* remove java21 API for backporting to 2.x

Signed-off-by: Heng Qian <[email protected]>

---------

Signed-off-by: Heng Qian <[email protected]>
(cherry picked from commit 0a6a2b0)

Co-authored-by: qianheng <[email protected]>
@b4sjoo b4sjoo added the v2.16.0 Issues targeting release v2.16.0 label Jul 26, 2024
phvalguima added a commit to canonical/opensearch-snap that referenced this pull request Aug 5, 2024
This PR backports upstream fix for ML Commons: opensearch-project/ml-commons#2675

To be added to our distribution.

Given we are currently targeting upstream branch 2.14, instead of upstream tag 2.14.0.0, this build is using a new release from our build from source: 2.14.0-ubuntu1.
phvalguima added a commit to canonical/opensearch-snap that referenced this pull request Aug 5, 2024
This PR backports upstream fix for ML Commons: opensearch-project/ml-commons#2675

Given we are currently targeting upstream branch 2.14, instead of upstream tag 2.14.0.0, this build is using a new release from our build from source: 2.14.0-ubuntu1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants