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

SOLR-17113: fix backup response in case of exception #2334

Merged
merged 3 commits into from
Mar 27, 2024

Conversation

stforek
Copy link
Contributor

@stforek stforek commented Mar 5, 2024

Return exception as a json object instead of an array

https://issues.apache.org/jira/browse/SOLR-17113

Description

curl 'http://localhost:8983/solr/core/replication?command=details'

Returns exception as an array instead of an object:

{
    "responseHeader":{
      "status":0,
      "QTime":1
    },
    "status":"OK",
    "details":{
      //redacted
      "backup":["exception","The specified bucket does not exist."]
    }
  }

Solution

Replaced NamedList with SimpleOrderedMap which encodes as an object in json response.

Tests

Verified locally only. It looks that implementing a test case for this use case is difficult.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

Return exception as a json object instead of an array
Copy link
Contributor

@cpoerschke cpoerschke left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution!

Would you like to add a solr/CHANGES.txt entry, in the 9.6.0 Bug Fixes section?

Copy link
Contributor

@gerlowskija gerlowskija left a comment

Choose a reason for hiding this comment

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

LGTM - sorry for the delay in reviewing this, I've been offline for a few weeks!

re: merging - I don't want to step on your toes @cpoerschke , were you planning to test and merge, or should I? (Happy to do it, if needed)

@cpoerschke
Copy link
Contributor

... re: merging - I don't want to step on your toes @cpoerschke , were you planning to test and merge, or should I? (Happy to do it, if needed)

Thanks for checking! Feel free to go ahead.

@gerlowskija gerlowskija merged commit 883fd6d into apache:main Mar 27, 2024
2 of 3 checks passed
gerlowskija added a commit that referenced this pull request Mar 27, 2024
---------

Co-authored-by: Przemyslaw Ciezkowski <pciezkowski@box.com>
Co-authored-by: Jason Gerlowski <gerlowskija@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants