igiguere commented on code in PR #3955:
URL: https://github.com/apache/solr/pull/3955#discussion_r2632564569
##########
solr/core/src/java/org/apache/solr/cli/CLIUtils.java:
##########
@@ -249,12 +247,7 @@ public static String getZkHost(CommandLine cli) throws
Exception {
try (SolrClient solrClient = getSolrClient(cli)) {
// hit Solr to get system info
- NamedList<Object> systemInfo =
- solrClient.request(
- new GenericSolrRequest(SolrRequest.METHOD.GET,
CommonParams.SYSTEM_INFO_PATH));
-
- // convert raw JSON into user-friendly output
- Map<String, Object> status = StatusTool.reportStatus(systemInfo,
solrClient);
+ Map<String, Object> status = StatusTool.reportStatus(solrClient);
Review Comment:
There are 17 calls to CLIUtils.getZkHost(CommandLine), where we find this
call to StatusTool.reportStatus(SolrClient).
That's half the *Tool found in org.apache.solr.cli. That means CLIUtils
serves as a "bridge" between tools (StatusTool and the rest of them).
Personally, I think it makes sense that the StatusTool would be the one
reporting the status, rather than have it depend on the CLIUtils to essentially
do the status reporting job (i.e.: CLIUtils.reportStatus).
Otherwise, if we don't want CLIUtils to call StatusTool, then, we have to
add a call to StatusTool.reportStatus(SolrClient) in each of the 17 locations,
and for each, find the ZK host currently provided by
CLIUtils.getZkHost(CommandLine).
In a nutchell : I would not change this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
