SOLR-18118: Add in --prompt-inputs to bin/solr start -e cloud to programatically respond to input prompts#4127
Conversation
|
@rahulgoswami could you check out the |
malliaridis
left a comment
There was a problem hiding this comment.
--prompt=2,8983,8984,"mycollection",2,2,_default
This seems kind of cryptic and the order can easily be messed up. It is very easy to make mistakes, imagine following scenarios:
User confuses the order of ports and shards / replicas, the prompt:
--prompts=2,2,2,"mycollection",8983,8984,_defautOr the user used
--prompts=2,8983,8984,"mycollection",2,2,_defautand now comes up with "oh, lets try 3 instead of 2 nodes" and just updates the prompts like
--prompts=3,8983,8984,"mycollection",2,2,_defautor user may think using it like
--prompts 3 8983 8984 "mycollection" 2 2 "_default"So I feel like there are too many wrong inputs possible.
I would rather prefer more params where we clearly distinguish the input data too. If it is supposed for automation, we still should maintain the readability and avoid "magic numbers". I would personally prefer options like --node-count=2, --node-ports=8983,8984 and so on, so that it is at least clear what the user is setting and the order doesn't matter.
Although not exactly the same, clig.dev says:
If you’ve got two or more arguments for different things, you’re probably doing something wrong.
source
The windows script also fails with
> bin\solr.cmd start -e cloud --prompts 3,8983,8984,8985,"my_collection",6,3,"_default"
Invalid command-line option: 3
Usage: solr start [-f] [--user-managed] [...]It seems that CMD is splitting the params by commas, so it fails after trying to parse the argument 3. In order to make it work I had to make the following changes:
:set_prompt
set "PASS_TO_RUN_EXAMPLE=--prompts %2 !PASS_TO_RUN_EXAMPLE!"
SHIFT # <-- Add a second shift to remove the argument of --prompts
SHIFT
goto parse_argsAdditionally update --prompt to --prompts (see other comments), and then run the command as
> bin\solr.cmd start -e cloud --prompts "3,8983,8984,8985,my_collection,6,3,_default" so that it is not split by comma (annoying, I know).
With these changes it worked on Windows, but I do not like it so much. The texts that are printed give the false impression that the user is supposed to make some prompt (they should be suppressed instead).
The --prompts is also specific to --e cloud, is it supposed to work for the other examples as well? If not, should it be listed in the start command, if it is only valid for -e cloud?
|
yeah, so there are some weasel words in the description of |
|
Partial review since I haven't had a chance to try out the .cmd changes yet. I have to agree with @malliaridis assessment that there is no way one would remember this order. On the flip side, introducing a CLI parameter for each individual value feels like an overkill. I do feel this can be mitigated through documentation both in the .adoc and especially in the CLI options itself. May I also suggest changing "--prompt" to something like "--default-cloud-options" or "--cloud-options-list" ? The current choice of name wasn't intuitive to me at all until I looked at the diff. |
| + | ||
| For example, when using the "cloud" example, can start a three node cluster on specific ports: | ||
| + | ||
| *Example*: `bin/solr start -e cloud --prompts 3,9000,9001,9002,"mycollection",2,2,_defaults` |
There was a problem hiding this comment.
Can we document the order ?
[number of nodes, comma separated port for each node, collection-name...]
There was a problem hiding this comment.
what if we call it --example-prompts? Or --example-inputs? The docs say --no-prompt is for anything that takes user supplied prompts... --user-inputs?.
There was a problem hiding this comment.
--user-inputs could work. --prompt-inputs could be another candidate?
There was a problem hiding this comment.
is "prompts" too overloaded in our AI times? How about --example-inputs to relate it to our examples?
There was a problem hiding this comment.
though of course then we have --no-prompts... Now I come back to --prompt-inputs.
There was a problem hiding this comment.
The way I think about this...someone who is trying solr for the first time won't know that a --no-prompt or --prompt-inputs option exists unless they go looking, and when they do, both terms might sound equally ambiguous at first in today's AI world. But then we already have a --no-prompt option and we are not going back. TLDR; It doesn't matter for new users. They should come around once they read the option description and then hopefully both options make sense in conjunction.
For those familiar with running solr and playing with examples, they are likely aware of the --no-prompt option already. And for them, --prompt-inputs should be a relatable extension. Let's go with this one?
There was a problem hiding this comment.
I like your reasonining @rahulgoswami and can get behind it! --prompt-inputs it is!
| .argName("VALUES") | ||
| .desc( | ||
| "Provide comma-separated values for prompts. Same as --no-prompt but uses provided values instead of defaults. " | ||
| + "Example: --prompts 3,8983,8984,8985,\"gettingstarted\",2,2,_default") |
There was a problem hiding this comment.
Can we document the order here too like in the .adoc? I feel documenting here might be more important, since if my usage pattern is any indicator of a wider usage pattern, users might prefer the CLI options telling them the expectation rather than them having to look up documentation for the same.
There was a problem hiding this comment.
or could --prompts take a json {"nodes":3,"port":"8983"}? Or take name of a properties file? It's an expert option so need not be beautiful
There was a problem hiding this comment.
yeah.. I worry about building something that is more code then the value... and, the number of items changes depending on inputs. so if it's one node, you are 1,8983,gettingstarted,2,2,_default, and if it's four, 4,9000,9001,9003,9002,"gettingstarted\",2,2,_default. Now imagaine we add some more prompts, then it will all change...
There was a problem hiding this comment.
Passing a formatted json in CLI could quickly get tricky with the quote escaping etc, especially on Windows. I like the properties file idea. Somewhat like we have for basic auth in solr.in.sh and solr.in.cmd with "SOLR_AUTHENTICATION_OPTS" (https://solr.apache.org/guide/solr/latest/deployment-guide/basic-authentication-plugin.html#using-the-solr-control-script-with-basic-auth). It can take the actual credentials as well as path to a file.
I am ok if the properties file enhancement comes as a second wave (one can hope!) and we could still use it against the same --prompts param.
|
Maybe we should add a |
solr/bin/solr.cmd
Outdated
| IF "%1"=="--jettyconfig" goto set_addl_jetty_config | ||
| IF "%1"=="-y" goto set_noprompt | ||
| IF "%1"=="--no-prompt" goto set_noprompt | ||
| IF "%1"=="--prompt"s goto set_prompts |
There was a problem hiding this comment.
misplaced quote in "--prompts" breaks command
solr/bin/solr.cmd
Outdated
| @echo. | ||
| @echo -y/--no-prompt Don't prompt for input; accept all defaults when running examples that accept user input | ||
| @echo. | ||
| @echo --prompts "values" Don't prompt for input; comma delimited list of inputs read when running examples that accept user input. |
There was a problem hiding this comment.
Suggest documenting the order of inputs with an example
--prompt-inputs "number _of_nodes, list_of_ports, collection_name, number_of_shards, number_of_replicas_per_shard, configuration_name"
eg: --prompt-inputs "3,8983,8984,8985,gettingstarted,2,2,_default"
| echo(nextInput); | ||
| } | ||
| } else { | ||
| // Reading from user input - use nextLine() |
There was a problem hiding this comment.
Suggest moving echo(prompt); from the first line of the method to over here. It would be fair to expect the CLI output for --prompt-inputs to be similar to that of --no-prompt, aka without lines asking for inputs, since we are already providing them in one shot.
There was a problem hiding this comment.
I like seeing the lines because it helps me with debugging... I did defaultss instead of default for a configset name and this let me see the error.
| nextInput = s.hasNext() ? s.next() : null; | ||
| // Echo the value being used from prompts | ||
| if (nextInput != null) { | ||
| echo(nextInput); |
There was a problem hiding this comment.
Do we need to print here?
There was a problem hiding this comment.
I tried with and without and it looks much better with! Plus you can debug it. I was hoping that https://clig.dev/#interactivity would give some specific advice on formatting, but it didn't hold any wisdom...
|
THanks all for comments! In the not too distant future I'm rolling out some tests that compare user-managed (standalone) to cloud, and cloud 1 shard to cloud 2 shard, and I expect to power that using these inputs for set up. For user-managed (standalone) to cloud, I think I need a |
|
I'm going to merge this as I need it for perf testing. We can iterate on it if it turns out not to be as useful as I hope it will be. |
There was a problem hiding this comment.
Pull request overview
Adds a non-interactive way to run bin/solr start -e cloud by supplying ordered prompt answers on the command line, enabling scripted/automated cluster startup while keeping existing interactive behavior available.
Changes:
- Introduces
--prompt-inputsinRunExampleToolto read prompt answers from a comma-separated list. - Updates
bin/solrandbin/solr.cmdto accept and forward--prompt-inputs, and updates the Ref Guide. - Adds a CLI test covering the new flag and adds a changelog entry.
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc | Documents --prompt-inputs and reorganizes example/prompt options in the start command reference. |
| solr/core/src/test/org/apache/solr/cli/TestSolrCLIRunExample.java | Adds a new test exercising --prompt-inputs for the SolrCloud example. |
| solr/core/src/java/org/apache/solr/cli/RunExampleTool.java | Implements --prompt-inputs parsing and wiring into the SolrCloud example prompting flow. |
| solr/bin/solr.cmd | Adds Windows argument parsing and usage text for --prompt-inputs. |
| solr/bin/solr | Adds *nix argument parsing and usage text for --prompt-inputs. |
| changelog/unreleased/SOLR-18118.yml | Adds an unreleased changelog entry for the new flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
solr/solr-ref-guide/modules/deployment-guide/pages/solr-control-script-reference.adoc
Outdated
Show resolved
Hide resolved
| InputStream promptsStream = | ||
| new java.io.ByteArrayInputStream(promptsValue.getBytes(StandardCharsets.UTF_8)); |
There was a problem hiding this comment.
Using Scanner with useDelimiter(",") will skip empty tokens (e.g. a,,b), so users can’t "accept default" for a prompt in the middle while still supplying later values. Consider parsing with a CSV-aware parser or split(",", -1) to preserve empty fields so prompt positions stay aligned.
| InputStream promptsStream = | |
| new java.io.ByteArrayInputStream(promptsValue.getBytes(StandardCharsets.UTF_8)); | |
| // Preserve empty fields so that users can "accept default" for a prompt | |
| // while still providing values for later prompts. | |
| String[] promptTokens = promptsValue.split(",", -1); | |
| for (int i = 0; i < promptTokens.length; i++) { | |
| if (promptTokens[i].isEmpty()) { | |
| // Use a single space so that Scanner produces a token, which the | |
| // prompting logic can then trim() back to an empty string. | |
| promptTokens[i] = " "; | |
| } | |
| } | |
| String processedPromptsValue = String.join(",", promptTokens); | |
| InputStream promptsStream = | |
| new java.io.ByteArrayInputStream( | |
| processedPromptsValue.getBytes(StandardCharsets.UTF_8)); |
…ramatically respond to input prompts (apache#4127) Adds a non-interactive way to run bin/solr start -e cloud by supplying ordered prompt answers on the command line, enabling scripted/automated cluster startup while keeping existing interactive behavior available.
https://issues.apache.org/jira/browse/SOLR-18118
Description
Populate the prompts that bin/solr start -e cloud makes with preset answers. Prompts.... in the old school sense of the word ;-)
This would be MOST valuable if we could backport it to 9x, so we can test 9 against 10. However, even if it only makes it into 10.1, well, we can test 11 and 10 ;-)
Solution
A bit of refactoring in how we read in choices. A new
--prompt=2,8983,8984,"mycollection",2,2,_defaultsetting that provides comma delimited values.Tests
Added a test and manual testing.