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 #160: ACCUMULO-4490: Simplify Accumulo scripts and con...
Date Wed, 26 Oct 2016 22:26:34 GMT
Github user ctubbsii commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/160#discussion_r85225730
  
    --- Diff: core/src/main/java/org/apache/accumulo/core/Constants.java ---
    @@ -126,7 +126,8 @@
       public static final String EXPORT_INFO_FILE = "accumulo_export_info.txt";
     
       // Variables that will be substituted with environment vars in PropertyType.PATH values
    -  public static final Collection<String> PATH_PROPERTY_ENV_VARS = Collections.unmodifiableCollection(Arrays.asList("ACCUMULO_HOME",
"ACCUMULO_CONF_DIR"));
    +  public static final Collection<String> PATH_PROPERTY_ENV_VARS = Collections.unmodifiableCollection(Arrays.asList("ACCUMULO_BIN_DIR",
"ACCUMULO_LIB_DIR",
    --- End diff --
    
    I don't think it's necessary to enshrine new properties to be interpolated in the code.
I think we need to move away from this. We can preserve what's there, but it'd be better to
put people on a path away from using environment variables interpolated in code.
    
    When we bootstrap the config, we can substitute the properties, so we don't have to interpolate
them in code ourselves (though, interpolation may be a future option from a better configuration
library).
    
    We can retain the existing interpolations of ACCUMULO_HOME and ACCUMULO_CONF_DIR, but
maybe with warnings.


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