You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Balázs Donát Bessenyei <be...@cloudera.com> on 2016/09/15 17:01:51 UTC

Review Request 51918: FLUME-2993: Support environment variables in configuration files

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51918/
-----------------------------------------------------------

Review request for Flume.


Bugs: FLUME-2993
    https://issues.apache.org/jira/browse/FLUME-2993


Repository: flume-git


Description
-------

Flume does not currently support environment variable interpolation in the properties file configuration.

Enabling it would help with
* removing security credentials from config files
* help with copy-pastes in configuration files with multiple agents defined

It is arguably a best practice to store (some) config in the environment variables: https://12factor.net/config


Diffs
-----

  c/flume-ng-doc/sphinx/FlumeUserGuide.rst 0fecee6 
  c/flume-ng-node/pom.xml e33b566 
  c/flume-ng-node/src/main/java/org/apache/flume/node/PropertiesFileConfigurationProvider.java b428c9e 
  w/flume-ng-node/src/main/java/org/apache/flume/node/EnvVarResolverProperties.java PRE-CREATION 
  w/flume-ng-node/src/test/java/org/apache/flume/node/TestEnvVarResolverProperties.java PRE-CREATION 
  w/flume-ng-node/src/test/resources/flume-conf-with-envvars.properties PRE-CREATION 

Diff: https://reviews.apache.org/r/51918/diff/


Testing
-------

mvn clean install runs successfully

new unit tests were added


Thanks,

Bal�zs Don�t Bessenyei


Re: Review Request 51918: FLUME-2993: Support environment variables in configuration files

Posted by Balázs Donát Bessenyei <be...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51918/
-----------------------------------------------------------

(Updated Sept. 24, 2016, 8:26 p.m.)


Review request for Flume.


Bugs: FLUME-2993
    https://issues.apache.org/jira/browse/FLUME-2993


Repository: flume-git


Description (updated)
-------

Flume does not currently support environment variable interpolation in the properties file configuration.

Enabling it would help with:
* removing security credentials from config files
* help with copy-pastes in configuration files with multiple agents defined

It is arguably a best practice to store (some) config in the environment variables: https://12factor.net/config


Diffs
-----

  c/flume-ng-doc/sphinx/FlumeUserGuide.rst 0fecee6 
  c/flume-ng-node/pom.xml e33b566 
  c/flume-ng-node/src/main/java/org/apache/flume/node/PropertiesFileConfigurationProvider.java b428c9e 
  w/flume-ng-node/src/main/java/org/apache/flume/node/EnvVarResolverProperties.java PRE-CREATION 
  w/flume-ng-node/src/test/java/org/apache/flume/node/TestEnvVarResolverProperties.java PRE-CREATION 
  w/flume-ng-node/src/test/resources/flume-conf-with-envvars.properties PRE-CREATION 

Diff: https://reviews.apache.org/r/51918/diff/


Testing
-------

mvn clean install runs successfully

new unit tests were added


Thanks,

Bal�zs Don�t Bessenyei


Re: Review Request 51918: FLUME-2993: Support environment variables in configuration files

Posted by Balázs Donát Bessenyei <be...@cloudera.com>.

> On Sept. 24, 2016, 12:52 p.m., Lior Zeno wrote:
> > LGTM, should we enable this by default?

Thank you for the review!

I'm not against enabling it by default. I'd like to have more people's opinion about it before making the change.


- Bal�zs Don�t


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51918/#review150300
-----------------------------------------------------------


On Sept. 15, 2016, 5:01 p.m., Bal�zs Don�t Bessenyei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51918/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 5:01 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2993
>     https://issues.apache.org/jira/browse/FLUME-2993
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Flume does not currently support environment variable interpolation in the properties file configuration.
> 
> Enabling it would help with
> * removing security credentials from config files
> * help with copy-pastes in configuration files with multiple agents defined
> 
> It is arguably a best practice to store (some) config in the environment variables: https://12factor.net/config
> 
> 
> Diffs
> -----
> 
>   c/flume-ng-doc/sphinx/FlumeUserGuide.rst 0fecee6 
>   c/flume-ng-node/pom.xml e33b566 
>   c/flume-ng-node/src/main/java/org/apache/flume/node/PropertiesFileConfigurationProvider.java b428c9e 
>   w/flume-ng-node/src/main/java/org/apache/flume/node/EnvVarResolverProperties.java PRE-CREATION 
>   w/flume-ng-node/src/test/java/org/apache/flume/node/TestEnvVarResolverProperties.java PRE-CREATION 
>   w/flume-ng-node/src/test/resources/flume-conf-with-envvars.properties PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51918/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install runs successfully
> 
> new unit tests were added
> 
> 
> Thanks,
> 
> Bal�zs Don�t Bessenyei
> 
>


Re: Review Request 51918: FLUME-2993: Support environment variables in configuration files

Posted by Lior Zeno <li...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51918/#review150300
-----------------------------------------------------------



LGTM, should we enable this by default?

- Lior Zeno


On Sept. 15, 2016, 5:01 p.m., Bal�zs Don�t Bessenyei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51918/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2016, 5:01 p.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2993
>     https://issues.apache.org/jira/browse/FLUME-2993
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Flume does not currently support environment variable interpolation in the properties file configuration.
> 
> Enabling it would help with
> * removing security credentials from config files
> * help with copy-pastes in configuration files with multiple agents defined
> 
> It is arguably a best practice to store (some) config in the environment variables: https://12factor.net/config
> 
> 
> Diffs
> -----
> 
>   c/flume-ng-doc/sphinx/FlumeUserGuide.rst 0fecee6 
>   c/flume-ng-node/pom.xml e33b566 
>   c/flume-ng-node/src/main/java/org/apache/flume/node/PropertiesFileConfigurationProvider.java b428c9e 
>   w/flume-ng-node/src/main/java/org/apache/flume/node/EnvVarResolverProperties.java PRE-CREATION 
>   w/flume-ng-node/src/test/java/org/apache/flume/node/TestEnvVarResolverProperties.java PRE-CREATION 
>   w/flume-ng-node/src/test/resources/flume-conf-with-envvars.properties PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/51918/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install runs successfully
> 
> new unit tests were added
> 
> 
> Thanks,
> 
> Bal�zs Don�t Bessenyei
> 
>