janhoy commented on code in PR #2712:
URL: https://github.com/apache/solr/pull/2712#discussion_r1803440828


##########
solr/core/src/java/org/apache/solr/cli/StatusTool.java:
##########
@@ -65,72 +73,224 @@ public String getName() {
           .desc("Wait up to the specified number of seconds to see Solr 
running.")
           .build();
 
+  public static final Option OPTION_PORT =
+      Option.builder("p")
+          .longOpt("port")
+          .argName("PORT")
+          .required(false)
+          .hasArg()
+          .desc("Port on localhost to check status for")
+          .build();
+
+  public static final Option OPTION_SHORT =
+      Option.builder()
+          .longOpt("short")
+          .argName("SHORT")
+          .required(false)
+          .desc("Short format. Prints one URL per line for running instances")
+          .build();
+
   @Override
   public List<Option> getOptions() {
-    return List.of(
-        // The solr-url option is not exposed to the end user, and is
-        // created by the bin/solr script and passed into this command 
directly,
-        // therefore we don't use the SolrCLI.OPTION_SOLRURL.
-        Option.builder()
-            .argName("URL")
-            .longOpt("solr-url")
-            .hasArg()
-            .required(false)
-            .desc("Property set by calling scripts, not meant for user 
configuration.")
-            .build(),
-        OPTION_MAXWAITSECS);
+    return List.of(OPTION_SOLRURL, OPTION_MAXWAITSECS, OPTION_PORT, 
OPTION_SHORT);
   }
 
   @Override
   public void runImpl(CommandLine cli) throws Exception {
-    // Override the default help behaviour to put out a customized message 
that only list user
-    // settable Options.
-    if ((cli.getOptions().length == 0 && cli.getArgs().length == 0)
-        || cli.hasOption("h")
-        || cli.hasOption("help")) {
-      final Options options = new Options();
-      options.addOption(OPTION_MAXWAITSECS);
-      new HelpFormatter().printHelp("status", options);
-      return;
+    String solrUrl = cli.getOptionValue(OPTION_SOLRURL);
+    Integer port =
+        cli.hasOption(OPTION_PORT) ? 
Integer.parseInt(cli.getOptionValue(OPTION_PORT)) : null;
+    boolean shortFormat = cli.hasOption(OPTION_SHORT);
+    int maxWaitSecs = Integer.parseInt(cli.getOptionValue("max-wait-secs", 
"0"));
+
+    if (port != null && solrUrl != null) {
+      throw new IllegalArgumentException("Only one of port or url can be 
specified");
     }
 
-    int maxWaitSecs = Integer.parseInt(cli.getOptionValue("max-wait-secs", 
"0"));
-    String solrUrl = SolrCLI.normalizeSolrUrl(cli);
-    if (maxWaitSecs > 0) {
-      int solrPort = new URI(solrUrl).getPort();
-      echo("Waiting up to " + maxWaitSecs + " seconds to see Solr running on 
port " + solrPort);
-      try {
-        waitToSeeSolrUp(
-            solrUrl,
-            cli.getOptionValue(SolrCLI.OPTION_CREDENTIALS.getLongOpt()),
-            maxWaitSecs,
-            TimeUnit.SECONDS);
-        echo("Started Solr server on port " + solrPort + ". Happy searching!");
-      } catch (TimeoutException timeout) {
-        throw new Exception(
-            "Solr at " + solrUrl + " did not come online within " + 
maxWaitSecs + " seconds!");
+    if (solrUrl != null) {
+      // URL provided, do not consult local processes, as the URL may be remote
+      if (maxWaitSecs > 0) {
+        // Used by Windows start script when starting Solr
+        try {
+          waitForSolrUpAndPrintStatus(solrUrl, cli, maxWaitSecs);

Review Comment:
   I don't like the fact that the output text changes completely based on a 
flag like `--max-wait-secs`, but that's what I added now to get the windows 
print correct. @epugh I believe you added this at some point.
   
   I'd suggest that `--max-wait-secs` only triggers the wait-and-retry logic 
and still prints the JSON output. Then we could use the exit code (0 or 1) from 
the statusTool to choose what to print in `solr.cmd`. Comments?



-- 
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]

Reply via email to