Introduce unified entrypoint for CLI scripts#85821
Conversation
CLI scripts have a common infrastructure in that they call to the shared elasticsearch-cli shell script which launches them with the appropriate java command line. However, each underlying Java class must implement its own main method. This commit introduces a single main method to be shared by CLIs. The new CliToolLauncher takes in system properties to determine which tool is being run, and a new CliToolProvider SPI allows defining and finding the named tools. relates elastic#85758
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
@albertzaharovits Could you please double check the auto configure nodes changes here? AutoConfigureNode had a "--reconfigure", which appeared to only be used so that the same underly Command class could be used in two different scenarios. This change keeps the one Command class, but instantiates it in two different ways, depending on if it is called from bin/elasticsearch, or elasticsearch-reconfigure-node. |
|
Note: this was spun off from #85758 to decrease the size of that PR. Most of the changes here are mechanical, just boilerplate adding the provider classes. |
|
@elasticmachine run elasticsearch-ci/docs |
|
Some more suggestions for making reviewing easier. Sorry for the total number of lines. I think all of the Provider subclasses and META-INF/services files can be skipped, they are just boilerplate. The shell script changes were mechanical, so they should be easy to skim through. Changes to the individual Command subclasses were also mechanical: remove the main method and make the class package protected. The interesting files are really CliToolLauncher and CliToolProvider, which are very small. Hope that helps. |
| set ES_ADDITIONAL_CLASSPATH_DIRECTORIES=lib/tools/security-cli | ||
| set CLI_SCRIPT=%~0 | ||
| set CLI_LIBS=modules/x-pack-core,modules/x-pack-security,lib/tools/security-cli | ||
| call "%~dp0elasticsearch-cli.bat" "--reconfigure" ^ |
There was a problem hiding this comment.
The reconfigure option should not be passed here, right?
albertzaharovits
left a comment
There was a problem hiding this comment.
Besides https://github.com/elastic/elasticsearch/pull/85821/files#diff-4533746782f7e57dfcc0f2b37dfc394b525383628477bb4ba7a980452fc05317
LGTM wrt to the AutoConfigureNode changes. Indeed only the elasticsearch-reconfigure-node cli was supposed to pass in the --reconfigure option.
CLI scripts have a common infrastructure in that they call to the shared
elasticsearch-cli shell script which launches them with the appropriate
java command line. However, each underlying Java class must implement
its own main method.
This commit introduces a single main method to be shared by CLIs. The
new CliToolLauncher takes in system properties to determine which tool
is being run, and a new CliToolProvider SPI allows defining and finding
the named tools.
relates #85758