Github user ctubbsii commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/231#discussion_r105064808
  
    --- Diff: 
start/src/main/java/org/apache/accumulo/start/classloader/vfs/AccumuloVFSClassLoader.java
 ---
    @@ -171,8 +167,27 @@ public void run() {
         return classpath.toArray(new FileObject[classpath.size()]);
       }
     
    +  protected static String addToClasspath(String classpath, String 
addition) {
    +    boolean cpExists = classpath != null && !classpath.isEmpty();
    +    boolean adExists = addition != null && !addition.isEmpty();
    +    if (cpExists && adExists) {
    +      return classpath + "," + addition;
    +    } else if (cpExists) {
    +      return classpath;
    +    } else if (adExists) {
    +      return addition;
    +    }
    +    return "";
    +  }
    +
       private static ReloadingClassLoader createDynamicClassloader(final 
ClassLoader parent) throws FileSystemException, IOException {
    -    String dynamicCPath = 
AccumuloClassLoader.getAccumuloString(DYNAMIC_CLASSPATH_PROPERTY_NAME, 
DEFAULT_DYNAMIC_CLASSPATH_VALUE);
    +    String dynamicClasspathJvm = 
System.getProperty(DYNAMIC_CLASSPATHS_JVM_PROPERTY, "");
    +    String dynamicClasspathSite = 
AccumuloClassLoader.getAccumuloString(DYNAMIC_CLASSPATHS_SITE_PROPERTY, "");
    +    String dynamicCPath = addToClasspath("", dynamicClasspathJvm);
    +    dynamicCPath = addToClasspath(dynamicCPath, dynamicClasspathSite);
    +    log.info("JVM property {} = {}", DYNAMIC_CLASSPATHS_JVM_PROPERTY, 
dynamicClasspathJvm);
    +    log.info("Config property {} = {}", DYNAMIC_CLASSPATHS_SITE_PROPERTY, 
dynamicClasspathSite);
    +    log.info("Derived dynamic classpath = {}", dynamicCPath);
    --- End diff --
    
    This is a lot of added behavioral complexity. Part of the reason I thought 
we were trying to move away from reliance on environment values, is because of 
the behavioral complexity we had to incorporate in code in order to handle 
those environmental settings. Switching from an environment variable to a 
system property doesn't really achieve this goal, if we're including a bunch of 
new code to handle the special circumstances of that property.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to