solr icon indicating copy to clipboard operation
solr copied to clipboard

SOLR-17285: Move RemoteSolrException to SolrClient in v10

Open samuelrivascoding opened this issue 1 year ago • 1 comments

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

Description

Moved RemoteSolrException to SolrClient from BaseHttpSolrClient

Solution

Moved RemoteSolrException class to SolrClient. Manually changed import statements from BaseHttpSolrClient to SolrClient if is not needed anymore for RemoteSolrException. Changed BaseHttpSolrClient.RemoteSolrException to SolrClient.RemoteSolrException

Tests

OUTPUT-org.apache.solr.cloud.HttpPartitionOnCommitTest.txt OUTPUT-org.apache.solr.cloud.ReplaceNodeTest.txt OUTPUT-org.apache.solr.handler.TestSolrConfigHandlerConcurrent.txt OUTPUT-org.apache.solr.pkg.TestPackages.txt

I ran ./gradlew dev. It worked fine ./gradlew check did not work. It said there were failing tests on unrelated files

Checklist

Please review the following and check all that apply:

  • [x] 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)
  • [x] I have developed this patch against the main branch.
  • [x] I have run ./gradlew check.
  • [x] I have added tests for my changes.
  • [ ] I have added documentation for the Reference Guide

samuelrivascoding avatar Jul 23 '24 20:07 samuelrivascoding

I am looking at BaseHttpSolrClient again, and I see that RemoteExecutionException is also something that'll need to be moved. It's not used in many places. I think it's reasonable to include that in the scope of this JIRA issue and PR; WDYT? But I don't love the name "RemoteExecutionException" and I'm not even convinced it needs to exist; like couldn't RemoteSolrException take a payload? Publishing it to SolrClient would give it undeserved visibility IMO. Hmmm. On the other hand, based on how it's used, this is so internal that maybe it should simply move to HttpSolrClientBase (which is the successor to BaseHttpSolrClient). I welcome your thoughts.

dsmiley avatar Jul 23 '24 22:07 dsmiley

This PR has had no activity for 60 days and is now labeled as stale. Any new activity or converting it to draft will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. Thank you for your contribution!

github-actions[bot] avatar Sep 22 '24 00:09 github-actions[bot]

@samuelrivascoding how shall I credit you (name you) in CHANGES.txt? If I don't hear back, it'll be literally that.

dsmiley avatar Sep 24 '24 02:09 dsmiley

Will merge as soon as Monday. I also plan to handle RemoteSolrException in a follow-up.

dsmiley avatar Sep 28 '24 21:09 dsmiley