dsmiley commented on a change in pull request #560:
URL: https://github.com/apache/solr/pull/560#discussion_r792307699



##########
File path: solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
##########
@@ -56,9 +52,12 @@
 public class PackageLoader implements Closeable {
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   public static final String LATEST = "$LATEST";
-  public static final String LOCAL_PACKAGES_DIR = 
System.getProperty("solr.packages.local.dir");
-  public static final String LOCAL_PACKAGES_WHITELIST = 
System.getProperty("solr.packages.local.whitelist", "");
+  public static final String LPD = "solr.packages.local.dir";

Review comment:
       Expand the acronym please.  I didn't know what it was at the spot that 
referenced it.

##########
File path: solr/core/src/test/org/apache/solr/pkg/TestLocalPackages.java
##########
@@ -12,33 +12,75 @@
 import org.apache.solr.cloud.MiniSolrCloudCluster;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.common.util.Utils;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.solr.pkg.PackageLoader.LPD;
+import static org.apache.solr.pkg.PackageLoader.SOLR_PACKAGES_LOCAL_ENABLED;
+
 public class TestLocalPackages extends SolrCloudTestCase {
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  public void testLocalPackagesAsDir() throws Exception {
+    String PKG_NAME = "mypkg";
+    String jarName = "mypkg1.jar";
+    String COLLECTION_NAME = "testLocalPkgsColl";
+    String localPackagesDir = "testpkgdir";
+    System.setProperty(SOLR_PACKAGES_LOCAL_ENABLED, PKG_NAME);
+    System.setProperty(LPD, localPackagesDir);
+    MiniSolrCloudCluster cluster =
+            configureCluster(4)

Review comment:
       Do we need a cluster this big to test this?

##########
File path: solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
##########
@@ -115,6 +120,36 @@ private void loadLocalPackages(List<String> 
enabledPackages) {
     }
   }
 
+  /**If a directory with no local_packages.json is provided,
+   * each sub directory that contains one or more jar files
+   * will be treated as a package and each jar file in that
+   * directory is added to the package classpath
+   */
+  private PackageAPI.Packages readDirAsPackages(Path packagesPath) {
+    PackageAPI.Packages localDirAsPackages = new PackageAPI.Packages();
+    packagesPath.toFile().list((dir, pkgName) -> {
+      File subDir = new File(dir, pkgName);

Review comment:
       Let's not use java.io.File and instead use Path, wherever possible.




-- 
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: issues-unsubscr...@solr.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to