epugh commented on code in PR #4695:
URL: https://github.com/apache/solr/pull/4695#discussion_r3705598941
##########
solr/core/src/java/org/apache/solr/cli/AssertTool.java:
##########
@@ -143,6 +143,25 @@ public class AssertTool extends ToolBase {
.longOpt("exitcode")
.get();
+ /**
+ * Parameters for the assert command, independent of the command line
parser. URL values are the
Review Comment:
thanks for hte comment
##########
solr/core/src/java/org/apache/solr/cli/RunExampleTool.java:
##########
@@ -237,14 +258,76 @@ public void runImpl(CommandLine cli) throws Exception {
this.urlScheme = cli.getOptionValue(URL_SCHEME_OPTION, "http");
String exampleType = cli.getOptionValue(EXAMPLE_OPTION);
- serverDir = Path.of(cli.getOptionValue(SERVER_DIR_OPTION));
+ initDirs(
+ cli.getOptionValue(SERVER_DIR_OPTION),
+ cli.getOptionValue(SCRIPT_OPTION),
+ cli.getOptionValue(EXAMPLE_DIR_OPTION),
+ cli.getOptionValue(SOLR_HOME_OPTION),
+ exampleType);
+
+ echoIfVerbose(
Review Comment:
is there more then one tool that has a "verbose" type concept? If so, then
lets keep this. maybe someday "echoIfVerbose" uses a fun colour!
##########
solr/core/src/java/org/apache/solr/cli/PackageTool.java:
##########
@@ -129,149 +129,67 @@ public void runImpl(CommandLine cli) throws Exception {
String cmd = cli.getArgs()[0];
- try (SolrClient solrClient = CLIUtils.getSolrClient(cli, true)) {
+ try (SolrClient solrClient =
+ CLIUtils.getSolrClient(
+ solrUrl,
cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION), true)) {
packageManager = new PackageManager(runtime, solrClient, solrUrl,
zkHost);
try {
repositoryManager = new RepositoryManager(solrClient,
packageManager);
+ // Dispatches to a parser-independent method per sub-command
switch (cmd) {
case "add-repo":
- String repoName = cli.getArgs()[1];
- String repoUrl = cli.getArgs()[2];
- repositoryManager.addRepository(repoName, repoUrl);
- printGreen("Added repository: " + repoName);
+ addRepo(cli.getArgs()[1], cli.getArgs()[2]);
break;
case "add-key":
- String keyFilename = cli.getArgs()[1];
- Path path = Path.of(keyFilename);
- repositoryManager.addKey(Files.readAllBytes(path),
path.getFileName().toString());
+ addKey(Path.of(cli.getArgs()[1]));
break;
case "list-installed":
- printGreen("Installed packages:\n-----");
- for (SolrPackageInstance pkg :
packageManager.fetchInstalledPackageInstances()) {
- printGreen(pkg);
- }
+ listInstalled();
break;
case "list-available":
- printGreen("Available packages:\n-----");
Review Comment:
while I will be sad to see the fun colour coding go away, this also brings
PackageTool in line with our other tools.. Maybe in the future we bring back
fun colours across all the tools
##########
solr/core/src/java/org/apache/solr/cli/PackageTool.java:
##########
@@ -292,6 +210,122 @@ public void runImpl(CommandLine cli) throws Exception {
}
}
+ private void addRepo(String repoName, String repoUrl) throws Exception {
+ repositoryManager.addRepository(repoName, repoUrl);
+ printGreen("Added repository: " + repoName);
+ }
+
+ private void addKey(Path keyFile) throws Exception {
+ repositoryManager.addKey(Files.readAllBytes(keyFile),
keyFile.getFileName().toString());
+ }
+
+ private void listInstalled() throws Exception {
+ printGreen("Installed packages:\n-----");
Review Comment:
I would just get rid of `printGreen` at this point... Let's make this tool
owrk like the others.
##########
solr/core/src/java/org/apache/solr/cli/StreamTool.java:
##########
@@ -403,22 +439,22 @@ public StreamComparator getStreamSort() {
}
}
- static String[] getOutputFields(CommandLine cli) {
- if (cli.hasOption(FIELDS_OPTION)) {
-
- String fl = cli.getOptionValue(FIELDS_OPTION);
- String[] flArray = fl.split(",");
- String[] outputHeaders = new String[flArray.length];
-
- for (int i = 0; i < outputHeaders.length; i++) {
- outputHeaders[i] = flArray[i].trim();
- }
-
- return outputHeaders;
-
- } else {
+ /**
+ * @param fl raw comma-separated list of fields, or null
+ * @return the trimmed field names, or null if no fields were given
+ */
+ static String[] getOutputFields(String fl) {
Review Comment:
much nicer! why we passed in an entire CLI?
--
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]
