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

    https://github.com/apache/accumulo/pull/236#discussion_r108526740
  
    --- Diff: 
start/src/main/java/org/apache/accumulo/start/classloader/AccumuloClassLoader.java
 ---
    @@ -44,21 +44,21 @@
     
     public class AccumuloClassLoader {
     
    -  public static final String CLASSPATH_PROPERTY_NAME = 
"general.classpaths";
    +  public static final String GENERAL_CLASSPATHS = "general.classpaths";
       public static final String MAVEN_PROJECT_BASEDIR_PROPERTY_NAME = 
"general.maven.project.basedir";
       public static final String DEFAULT_MAVEN_PROJECT_BASEDIR_VALUE = "";
     
    -  private static String SITE_CONF;
    +  private static URL accumuloConfigUrl;
       private static URLClassLoader classloader;
       private static final Logger log = 
LoggerFactory.getLogger(AccumuloClassLoader.class);
     
       static {
    -    String configFile = 
System.getProperty("org.apache.accumulo.config.file", "accumulo-site.xml");
    -    if (System.getenv("ACCUMULO_CONF_DIR") != null) {
    -      // accumulo conf dir should be set
    -      SITE_CONF = System.getenv("ACCUMULO_CONF_DIR") + "/" + configFile;
    +    String configFilename = 
System.getProperty("org.apache.accumulo.config.file", "accumulo-site.xml");
    +    accumuloConfigUrl = 
AccumuloClassLoader.class.getClassLoader().getResource(configFilename);
    +    if (accumuloConfigUrl == null) {
    +      log.warn("Failed to load Accumulo configuration '{}' from 
classpath", configFilename);
    --- End diff --
    
    @joshelser "We don't need this" isn't a sweeping statement. It's shorthand 
for "I think we don't need this". I just didn't think it necessary to preface 
all my opinions like that to achieve the desired level of reception.
    
    @mikewalch Your patch won't work. A leading slash is valid for class path 
resources, and you can't tell what the user intends by branching on that 
condition. The way log4j does it is with parsing a URI scheme, which is 
complicated, and bloaty, but is the only way you can add this feature. Again, I 
don't think the feature is worth adding, though.


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