> On 2012-05-05 10:51:23, Hari Shreedharan wrote: > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/BasicConfigurationConstants.java, > > line 36 > > <https://reviews.apache.org/r/5017/diff/1/?file=106958#file106958line36> > > > > This constant is defined twice. What is the difference between > > CONFIG_SOURCE_CHANNELSELECTOR_PREFIX and CONFIG_SELECTOR_PREFIX?
removed this. > On 2012-05-05 10:51:23, Hari Shreedharan wrote: > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java, > > line 57 > > <https://reviews.apache.org/r/5017/diff/1/?file=106962#file106962line57> > > > > This should be CONFIG_CHANNELS. good catch (phew)! fixed this. > On 2012-05-05 10:51:23, Hari Shreedharan wrote: > > /trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java, > > line 144 > > <https://reviews.apache.org/r/5017/diff/1/?file=106966#file106966line144> > > > > Not related to your change, but we might want to catch a > > NumberFormatException here. Same for even the eventSize getInteger call. Undersand your suggestion here and for the next comment Hari, but am inclined to defer this for later. - Arvind ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5017/#review7595 ----------------------------------------------------------- On 2012-05-05 06:22:46, Arvind Prabhakar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5017/ > ----------------------------------------------------------- > > (Updated 2012-05-05 06:22:46) > > > Review request for Flume, Brock Noland and Hari Shreedharan. > > > Summary > ------- > > This change: > * introduces a precondition check in Context.getSubProperties(String prefix) > method enforce that the prefix ends with a period, > * fixes the TestPropertiesFileConfigurationProvider to load the property file > which was previously not working, and fixes cases that are broken > * refactors some of the source (not all) to externalize the configuration > keys into separate constants class > > > This addresses bug FLUME-1181. > https://issues.apache.org/jira/browse/FLUME-1181 > > > Diffs > ----- > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/Context.java > 1334310 > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/BasicConfigurationConstants.java > PRE-CREATION > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfiguration.java > 1334310 > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java > 1334310 > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkGroupConfiguration.java > 1334310 > > /trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java > 1334310 > > /trunk/flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorFactory.java > 1334310 > > /trunk/flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java > 1334310 > > /trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java > PRE-CREATION > > /trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > 1334310 > > /trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > 1334310 > /trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java > 1334310 > > /trunk/flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java > 1334310 > > /trunk/flume-ng-node/src/test/java/org/apache/flume/conf/properties/TestPropertiesFileConfigurationProvider.java > 1334310 > /trunk/flume-ng-node/src/test/resources/flume-conf.properties 1334310 > > Diff: https://reviews.apache.org/r/5017/diff > > > Testing > ------- > > Ran all tests. The updated TestPropertiesFileConfigurationProvider catches > two issues now - the one with channel selector configuration not being set > correctly, and the other a similar issue with syslog source format > configuration. Both of these issues have been fixed with the changes. > > Also done some manual validation of the system with a few simple scenarios. > > > Thanks, > > Arvind > >
