accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ctubbsii <...@git.apache.org>
Subject [GitHub] accumulo pull request #231: ACCUMULO-4596 Remove env variable from general.d...
Date Thu, 09 Mar 2017 01:02:18 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message