SOLR-18118: Add in --prompts to bin/solr start -e cloud to automatically respond to prompts#4127
SOLR-18118: Add in --prompts to bin/solr start -e cloud to automatically respond to prompts#4127epugh wants to merge 8 commits intoapache:mainfrom
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 misleading at first in today's AI world. But then we already have a --no-prompt option and we are not going back. TLDR; 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.
| .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 |
| 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
| @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.
| 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?
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.