----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4508/#review7163 -----------------------------------------------------------
Functionally fine, just a couple of niggles flume-ng-configuration/src/main/java/org/apache/flume/conf/source/ExecSourceConfiguration.java <https://reviews.apache.org/r/4508/#comment15817> This should probably be a constant flume-ng-configuration/src/main/java/org/apache/flume/conf/source/ExecSourceConfiguration.java <https://reviews.apache.org/r/4508/#comment15816> I'm pretty sure that now we have a dedicated class for the configuration we don't need a subclass to hold the constants? flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java <https://reviews.apache.org/r/4508/#comment15818> For consistency I think this should go into the configuration and throw a ConfigurationException - Juhani On 2012-03-27 06:26:30, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4508/ > ----------------------------------------------------------- > > (Updated 2012-03-27 06:26:30) > > > Review request for Flume. > > > Summary > ------- > > ExecSource configuration > > > This addresses bug FLUME-1058. > https://issues.apache.org/jira/browse/FLUME-1058 > > > Diffs > ----- > > > flume-ng-configuration/src/main/java/org/apache/flume/conf/source/ExecSourceConfiguration.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java dbf79e0 > > flume-ng-core/src/main/java/org/apache/flume/source/ExecSourceConfigurationConstants.java > 73c985e > > Diff: https://reviews.apache.org/r/4508/diff > > > Testing > ------- > > > Thanks, > > Hari > >
