solr icon indicating copy to clipboard operation
solr copied to clipboard

Published 20 hours ago • verified publisher icon apache

SOLR-17501: Move out CLI utils from SolrCLI

Open malliaridis opened this issue 1 year ago • 3 comments

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

Description

To reduce the scope and dependencies to SolrCLI.java, utility / helper methods should be extracted from SolrCLI into a separate class. This way, SolrCLI acts only as an entrypoint and CLI initiation class.

Solution

Several static methods and attributes have been moved to CLIUtils.java. Commit history allows backporting of changes to 9.X.

Tests

Please describe the tests you've developed or run to confirm this patch implements the feature or solves the problem.

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, not available for branches on forks living under an organisation)
  • [x] 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

malliaridis avatar Oct 06 '24 15:10 malliaridis

Is this able to backwards ported to 9x? Seems good stuff!

epugh avatar Oct 17 '24 20:10 epugh

Is this able to backwards ported to 9x? Seems good stuff!

I think I could create a separate PR for backporting if we want these changes in 9x as well. I think a cherry-pick won't do in this case as I was expecting.

malliaridis avatar Oct 18 '24 12:10 malliaridis

This PR is related and should be blocked by #2778 for now.

malliaridis avatar Oct 18 '24 12:10 malliaridis

I'll also wait for https://github.com/apache/solr/pull/2725 to be merged before completing thins one.

malliaridis avatar Nov 06 '24 15:11 malliaridis

When I added the tests I noticed a few cases where we may require to patch things. To be precise:

  • URI is very lenient and allows many invalid strings without throwing URISyntaxException. Using URL in portFromUrl (that is used currently only by StatusTool) may be a good improvement
  • checkCommunicationError is false if a SolrServerException is thrown with a different root cause (e.g. NPE).

I tried to avoid any changes unrelated to the ticket and we still have many untested methods in this part.

malliaridis avatar Nov 11 '24 10:11 malliaridis