You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/06/30 07:30:33 UTC

[GitHub] [lucene-solr] chatman opened a new pull request #1631: SOLR-14599: Package manager support for cluster level plugins

chatman opened a new pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631


   Usage:
       bin/solr package deploy <packagename> -y -cluster
       bin/solr package deploy <packagename>:<new-version> -y -cluster --update
       bin/solr package undeploy <packagename> -y -cluster
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#discussion_r447531855



##########
File path: solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
##########
@@ -147,11 +179,37 @@ private void ensureCollectionsExist(List<String> collections) {
     }
   }
   
-  @SuppressWarnings({"unchecked"})
   private boolean deployPackage(SolrPackageInstance packageInstance, boolean pegToLatest, boolean isUpdate, boolean noprompt,
-      List<String> collections, String[] overrides) {
-    List<String> previouslyDeployed =  new ArrayList<>(); // collections where package is already deployed in
+      List<String> collections, boolean shouldDeployClusterPlugins, String[] overrides) {
 
+    // Install plugins of type "cluster"
+    boolean cluasterSuccess = deployClusterPackage(packageInstance, isUpdate, noprompt, shouldDeployClusterPlugins,

Review comment:
       typo? 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] dsmiley commented on a change in pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
dsmiley commented on a change in pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#discussion_r450609479



##########
File path: solr/core/src/java/org/apache/solr/util/PackageTool.java
##########
@@ -149,7 +149,8 @@ protected void runImpl(CommandLine cli) throws Exception {
                 String version = parsedVersion.second();
                 boolean noprompt = cli.hasOption('y');
                 boolean isUpdate = cli.hasOption("update") || cli.hasOption('u');
-                packageManager.deploy(packageName, version, PackageUtils.validateCollections(cli.getOptionValue("collections").split(",")), cli.getOptionValues("param"), isUpdate, noprompt);
+                String collections[] = cli.hasOption("collections")? PackageUtils.validateCollections(cli.getOptionValue("collections").split(",")): new String[] {};

Review comment:
       Please don't use C-style array declarations.  IMO our pre-commit ought to be enhanced to not allow this




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] noblepaul commented on pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#issuecomment-651707605


   +1


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] chatman commented on a change in pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#discussion_r447577311



##########
File path: solr/core/src/test-files/runtimecode/MyPlugin.java
##########
@@ -1,43 +0,0 @@
-/*

Review comment:
       Fixed. This file was causing problems in Eclipse. We should deal with it separately.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#discussion_r447530656



##########
File path: solr/core/src/test-files/runtimecode/MyPlugin.java
##########
@@ -1,43 +0,0 @@
-/*

Review comment:
       This file is accidentally deleted




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] chatman closed pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
chatman closed pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] noblepaul commented on a change in pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#discussion_r447534197



##########
File path: solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
##########
@@ -244,24 +306,114 @@ private boolean deployPackage(SolrPackageInstance packageInstance, boolean pegTo
       }
     }
 
+    if (previouslyDeployed.isEmpty() == false) {
+      PackageUtils.printRed("Already Deployed on " + previouslyDeployed + ", package: " + packageInstance.name + ", version: " + packageInstance.version);
+    }
+
     List<String> deployedCollections = collections.stream().filter(c -> !previouslyDeployed.contains(c)).collect(Collectors.toList());
+    return new Pair<List<String>, List<String>>(deployedCollections, previouslyDeployed);
+  }
 
-    boolean success = true;
-    if (deployedCollections.isEmpty() == false) {
-      // Verify that package was successfully deployed
-      success = verify(packageInstance, deployedCollections);
-      if (success) {
-        PackageUtils.printGreen("Deployed on " + deployedCollections + " and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
+  @SuppressWarnings("unchecked")
+  private boolean deployClusterPackage(SolrPackageInstance packageInstance, boolean isUpdate, boolean noprompt,
+      boolean shouldDeployClusterPlugins, String[] overrides) {
+    boolean cluasterPluginFailed = false;
+
+    if (isUpdate) {
+      for (Plugin plugin: packageInstance.plugins) {
+        if (!shouldDeployClusterPlugins || "cluster".equalsIgnoreCase(plugin.type) == false) continue;
+        SolrPackageInstance deployedPackage = getPackagesDeployedAsClusterLevelPlugins().get(packageInstance.name);
+        if (deployedPackage == null) {
+          PackageUtils.printRed("Cluster level plugin " + plugin.name + " from package " + packageInstance.name + " not deployed. To deploy, remove the --update parameter.");
+          cluasterPluginFailed = true;
+          continue;
+        }
+        for (Map<String, String> pluginMeta: (List<Map<String, String>>)deployedPackage.getCustomData()) {
+          PackageUtils.printGreen("Updating this plugin: " + pluginMeta);
+          try {
+            String postBody = "{\"update\":{\"name\": \""+pluginMeta.get("name")+"\","
+                + " \"class\": \""+pluginMeta.get("class")+"\", \"version\": \""+packageInstance.version+"\", \"path-prefix\": \""+pluginMeta.get("path-prefix")+"\"}}";

Review comment:
       what if there is no `path-prefix` ?
   
   Use the POJO `PluginMeta` and it will take care of these things. You do not need to construct this json string
   




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] chatman commented on a change in pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#discussion_r447577400



##########
File path: solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
##########
@@ -147,11 +179,37 @@ private void ensureCollectionsExist(List<String> collections) {
     }
   }
   
-  @SuppressWarnings({"unchecked"})
   private boolean deployPackage(SolrPackageInstance packageInstance, boolean pegToLatest, boolean isUpdate, boolean noprompt,
-      List<String> collections, String[] overrides) {
-    List<String> previouslyDeployed =  new ArrayList<>(); // collections where package is already deployed in
+      List<String> collections, boolean shouldDeployClusterPlugins, String[] overrides) {
 
+    // Install plugins of type "cluster"
+    boolean cluasterSuccess = deployClusterPackage(packageInstance, isUpdate, noprompt, shouldDeployClusterPlugins,

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/packagemanager/PackageManager.java
##########
@@ -244,24 +306,114 @@ private boolean deployPackage(SolrPackageInstance packageInstance, boolean pegTo
       }
     }
 
+    if (previouslyDeployed.isEmpty() == false) {
+      PackageUtils.printRed("Already Deployed on " + previouslyDeployed + ", package: " + packageInstance.name + ", version: " + packageInstance.version);
+    }
+
     List<String> deployedCollections = collections.stream().filter(c -> !previouslyDeployed.contains(c)).collect(Collectors.toList());
+    return new Pair<List<String>, List<String>>(deployedCollections, previouslyDeployed);
+  }
 
-    boolean success = true;
-    if (deployedCollections.isEmpty() == false) {
-      // Verify that package was successfully deployed
-      success = verify(packageInstance, deployedCollections);
-      if (success) {
-        PackageUtils.printGreen("Deployed on " + deployedCollections + " and verified package: " + packageInstance.name + ", version: " + packageInstance.version);
+  @SuppressWarnings("unchecked")
+  private boolean deployClusterPackage(SolrPackageInstance packageInstance, boolean isUpdate, boolean noprompt,
+      boolean shouldDeployClusterPlugins, String[] overrides) {
+    boolean cluasterPluginFailed = false;
+
+    if (isUpdate) {
+      for (Plugin plugin: packageInstance.plugins) {
+        if (!shouldDeployClusterPlugins || "cluster".equalsIgnoreCase(plugin.type) == false) continue;
+        SolrPackageInstance deployedPackage = getPackagesDeployedAsClusterLevelPlugins().get(packageInstance.name);
+        if (deployedPackage == null) {
+          PackageUtils.printRed("Cluster level plugin " + plugin.name + " from package " + packageInstance.name + " not deployed. To deploy, remove the --update parameter.");
+          cluasterPluginFailed = true;
+          continue;
+        }
+        for (Map<String, String> pluginMeta: (List<Map<String, String>>)deployedPackage.getCustomData()) {
+          PackageUtils.printGreen("Updating this plugin: " + pluginMeta);
+          try {
+            String postBody = "{\"update\":{\"name\": \""+pluginMeta.get("name")+"\","
+                + " \"class\": \""+pluginMeta.get("class")+"\", \"version\": \""+packageInstance.version+"\", \"path-prefix\": \""+pluginMeta.get("path-prefix")+"\"}}";

Review comment:
       Fixed, thanks.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] chatman commented on pull request #1631: SOLR-14599: Package manager support for cluster level plugins

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #1631:
URL: https://github.com/apache/lucene-solr/pull/1631#issuecomment-652701845


   Merged. Thanks.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org