janhoy commented on code in PR #4695:
URL: https://github.com/apache/solr/pull/4695#discussion_r3996287071


##########
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:
   Any functional change will be separate JIRAs.



-- 
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]

Reply via email to