epugh commented on code in PR #4695:
URL: https://github.com/apache/solr/pull/4695#discussion_r3995950523
##########
solr/core/src/java/org/apache/solr/cli/AssertTool.java:
##########
@@ -143,6 +144,50 @@ public class AssertTool extends ToolBase {
.longOpt("exitcode")
.get();
+ /** One requested assertion. Multiple assertions may be requested in a
single invocation. */
+ sealed interface Assertion {
Review Comment:
I've never seen `sealed` actually used!
##########
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-----");
- for (SolrPackage pkg : repositoryManager.getPackages()) {
- printGreen(pkg.name + " \t\t" + pkg.description);
- for (SolrPackageRelease version : pkg.versions) {
- printGreen("\tVersion: " + version.version);
- }
- }
+ listAvailable();
break;
case "list-deployed":
if (cli.hasOption(COLLECTION_OPTION)) {
- String collection = cli.getOptionValue(COLLECTION_OPTION);
- Map<String, SolrPackageInstance> packages =
- packageManager.getPackagesDeployed(collection);
- printGreen("Packages deployed on " + collection + ":");
- for (String packageName : packages.keySet()) {
- printGreen("\t" + packages.get(packageName));
- }
+
listPackagesDeployedOnCollection(cli.getOptionValue(COLLECTION_OPTION));
} else {
// nuance that we use an arg here instead of requiring a
--package parameter with a
- // value
- // in this code path
- String packageName = cli.getArgs()[1];
- Map<String, String> deployedCollections =
- packageManager.getDeployedCollections(packageName);
- if (!deployedCollections.isEmpty()) {
- printGreen("Collections on which package " + packageName + "
was deployed:");
- for (String collection : deployedCollections.keySet()) {
- printGreen(
- "\t"
- + collection
- + "("
- + packageName
- + ":"
- + deployedCollections.get(collection)
- + ")");
- }
- } else {
- printGreen("Package " + packageName + " not deployed on any
collection.");
- }
+ // value in this code path
+ listCollectionsWithPackageDeployed(cli.getArgs()[1]);
}
break;
case "install":
- {
- Pair<String, String> parsedVersion =
parsePackageVersion(cli.getArgList().get(1));
- String packageName = parsedVersion.first();
- String version = parsedVersion.second();
- boolean success = repositoryManager.install(packageName,
version);
- if (success) {
- printGreen(packageName + " installed.");
- } else {
- printRed(packageName + " installation failed.");
- }
- break;
- }
+ install(cli.getArgList().get(1));
+ break;
case "deploy":
- {
- if (cli.hasOption(CLUSTER_OPTION) ||
cli.hasOption(COLLECTIONS_OPTION)) {
- Pair<String, String> parsedVersion =
parsePackageVersion(cli.getArgList().get(1));
- String packageName = parsedVersion.first();
- String version = parsedVersion.second();
- boolean noPrompt = cli.hasOption(NO_PROMPT_OPTION);
- boolean isUpdate = cli.hasOption(UPDATE_OPTION);
- String[] collections =
- cli.hasOption(COLLECTIONS_OPTION)
- ? PackageUtils.validateCollections(
-
cli.getOptionValue(COLLECTIONS_OPTION).split(","))
- : new String[] {};
- String[] parameters = cli.getOptionValues(PARAM_OPTION);
- packageManager.deploy(
- packageName,
- version,
- collections,
- cli.hasOption(CLUSTER_OPTION),
- parameters,
- isUpdate,
- noPrompt);
- } else {
- printRed(
- "Either specify --cluster to deploy cluster level
plugins or --collections <list-of-collections> to deploy collection level
plugins");
- }
- break;
+ if (cli.hasOption(CLUSTER_OPTION) ||
cli.hasOption(COLLECTIONS_OPTION)) {
+ deploy(
+ cli.getArgList().get(1),
+ cli.hasOption(CLUSTER_OPTION),
+ cli.getOptionValue(COLLECTIONS_OPTION),
+ cli.getOptionValues(PARAM_OPTION),
+ cli.hasOption(UPDATE_OPTION),
+ cli.hasOption(NO_PROMPT_OPTION));
+ } else {
+ printRed(
Review Comment:
I think we should either commit to getting rid of printRed and printGreen or
open a JIRA ticket as a follow up that says "Embrace printRed and printGreen
acorss all our tools" so we use them actively. Otherwise it's always just a
werid thing.
--
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]
