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.

Sign up for GitHub

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

Jump to bottom

fix error code #2120

Merged
merged 1 commit into from
Mar 12, 2024
Merged

fix error code #2120

merged 1 commit into from
Mar 12, 2024

Conversation

jngz-es
Copy link
Collaborator

@jngz-es jngz-es commented Feb 16, 2024

Description

Fix the error code when getting the opensearch exception.

Issues Resolved

[List any issues this PR will resolve]

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.

@jngz-es
fix error code
ff94252 
Signed-off-by: Jing Zhang <[email protected]>
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 01:18 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 01:18 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 01:18 — with GitHub Actions Inactive
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env February 16, 2024 01:18 — with GitHub Actions Failure
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 03:51 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 03:51 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 03:51 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 03:51 — with GitHub Actions Inactive
@codecov Codecov
Copy link

codecov bot commented Feb 16, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.87%. Comparing base (3b162db) to head (ff94252).
Report is 23 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff            @@
##               main    #2120   +/-   ##
=========================================
  Coverage     81.86%   81.87%           
  Complexity     5644     5644           
=========================================
  Files           543      543           
  Lines         22790    22791    +1     
  Branches       2333     2333           
=========================================
+ Hits          18658    18659    +1     
  Misses         3195     3195           
  Partials        937      937
Flag Coverage Δ
ml-commons 81.87% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 04:35 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 04:35 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 04:35 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 04:35 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:21 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:21 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:21 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:21 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:48 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:48 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env February 16, 2024 05:48 — with GitHub Actions Inactive
dhrubo-os
dhrubo-os reviewed Feb 16, 2024
View reviewed changes
plugin/src/main/java/org/opensearch/ml/rest/RestMLExecuteAction.java
@@ -130,7 +131,8 @@ private void sendResponse(RestChannel channel, MLExecuteTaskResponse response) t
}

private void reportError(final RestChannel channel, final Exception e, final RestStatus status) {
channel.sendResponse(new BytesRestResponse(status, ErrorMessageFactory.createErrorMessage(e, status.getStatus()).toString()));
ErrorMessage errorMessage = ErrorMessageFactory.createErrorMessage(e, status.getStatus());
Copy link
Collaborator

@dhrubo-os dhrubo-os Feb 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is it possible add a unit test for this?

@jngz-es jngz-es closed this Feb 21, 2024
@jngz-es jngz-es deleted the notfoundagent branch February 21, 2024 17:29
@joshuali925 joshuali925 mentioned this pull request Mar 4, 2024
Closed
@jngz-es jngz-es restored the notfoundagent branch March 12, 2024 18:49
@jngz-es jngz-es reopened this Mar 12, 2024
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env March 12, 2024 18:51 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env March 12, 2024 18:51 — with GitHub Actions Inactive
@jngz-es jngz-es temporarily deployed to ml-commons-cicd-env March 12, 2024 18:51 — with GitHub Actions Inactive
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 12, 2024 18:51 — with GitHub Actions Error
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 12, 2024 18:51 — with GitHub Actions Error
@jngz-es jngz-es had a problem deploying to ml-commons-cicd-env March 12, 2024 18:51 — with GitHub Actions Failure
@jngz-es jngz-es merged commit a8f983b into opensearch-project:main Mar 12, 2024
27 of 30 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 12, 2024
@jngz-es @github-actions
fix error code (#2120)
3ac4b30 
Signed-off-by: Jing Zhang <[email protected]>
(cherry picked from commit a8f983b)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Mar 12, 2024
Merged
dhrubo-os pushed a commit that referenced this pull request Mar 12, 2024
@opensearch-trigger-bot @jngz-es
fix error code (#2120) (#2196)
63c72d0 
Signed-off-by: Jing Zhang <[email protected]>
(cherry picked from commit a8f983b)

Co-authored-by: Jing Zhang <[email protected]>
joshuali925
joshuali925 reviewed Mar 12, 2024
View reviewed changes
plugin/src/main/java/org/opensearch/ml/rest/RestMLExecuteAction.java
@@ -130,7 +131,8 @@ private void sendResponse(RestChannel channel, MLExecuteTaskResponse response) t
}

private void reportError(final RestChannel channel, final Exception e, final RestStatus status) {
channel.sendResponse(new BytesRestResponse(status, ErrorMessageFactory.createErrorMessage(e, status.getStatus()).toString()));
ErrorMessage errorMessage = ErrorMessageFactory.createErrorMessage(e, status.getStatus());
channel.sendResponse(new BytesRestResponse(RestStatus.fromCode(errorMessage.getStatus()), errorMessage.toString()));
Copy link
Member

Choose a reason for hiding this comment

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

possible to add header "application/json; charset=UTF-8" in the response here or convert error to a XContentBuilder instead of errorMessage.toString()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I will send another pr to change it.

austintlee pushed a commit to austintlee/ml-commons that referenced this pull request Mar 18, 2024
@jngz-es @austintlee
fix error code (opensearch-project#2120)
955c91f 
Signed-off-by: Jing Zhang <[email protected]>
austintlee pushed a commit to austintlee/ml-commons that referenced this pull request Mar 19, 2024
@jngz-es @austintlee
fix error code (opensearch-project#2120)
1a5b2f6 
Signed-off-by: Jing Zhang <[email protected]>
@jngz-es jngz-es deleted the notfoundagent branch January 13, 2025 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhrubo-os dhrubo-os dhrubo-os left review comments

@joshuali925 joshuali925 joshuali925 left review comments

@ylwu-amzn ylwu-amzn ylwu-amzn approved these changes

@Zhangxunmt Zhangxunmt Zhangxunmt approved these changes

@b4sjoo b4sjoo Awaiting requested review from b4sjoo b4sjoo is a code owner

@model-collapse model-collapse Awaiting requested review from model-collapse model-collapse is a code owner

@rbhavna rbhavna Awaiting requested review from rbhavna rbhavna is a code owner

@zane-neo zane-neo Awaiting requested review from zane-neo zane-neo is a code owner

@austintlee austintlee Awaiting requested review from austintlee austintlee is a code owner

@HenryL27 HenryL27 Awaiting requested review from HenryL27 HenryL27 is a code owner

@sam-herman sam-herman Awaiting requested review from sam-herman

Assignees
No one assigned
Labels
backport 2.x
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jngz-es @joshuali925 @Zhangxunmt @ylwu-amzn @dhrubo-os