----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4502/#review6970 -----------------------------------------------------------
I finally had time to go through this huge patch. Great work! In general it looks good, with a small number of minor niggles. Hopefully we can get this in before more major changes take place on the trunk. One other issue I have is that there is a lot of non-DRY code that looks like editted copy-paste. It may be a bit much to fix now, but hopefully in the future it can be refactored. flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java <https://reviews.apache.org/r/4502/#comment15524> s/PropertiesFileConfgurationProvider/PropertiesFileConfigurationProvider/ flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java <https://reviews.apache.org/r/4502/#comment15523> s/configuation/configuration/ flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java <https://reviews.apache.org/r/4502/#comment15526> do you think you could rename this to channelName for code clarity? Same thing for the validateSource/Sinks flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java <https://reviews.apache.org/r/4502/#comment15527> ChannelType->SourceType flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java <https://reviews.apache.org/r/4502/#comment15528> ChannelType-> SinkType flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java <https://reviews.apache.org/r/4502/#comment15529> I don't think this doc applies for the validateGroups as there is only one type of group configuration and it is handled here flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelType.java <https://reviews.apache.org/r/4502/#comment15547> I believe you'll need to add in the new channels here - Juhani On 2012-04-13 21:52:25, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4502/ > ----------------------------------------------------------- > > (Updated 2012-04-13 21:52:25) > > > Review request for Flume. > > > Summary > ------- > > Main config component. > > > This addresses bug FLUME-1052. > https://issues.apache.org/jira/browse/FLUME-1052 > > > Diffs > ----- > > flume-ng-configuration/pom.xml PRE-CREATION > flume-ng-configuration/src/main/java/org/apache/flume/Context.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfigurationFactory.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/ConfigurationException.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfigurationError.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfigurationErrorType.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelSelectorConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelSelectorType.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/ChannelType.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkGroupConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkProcessorConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkProcessorType.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkType.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceType.java > PRE-CREATION > flume-ng-core/pom.xml 37fb112 > flume-ng-core/src/main/java/org/apache/flume/ChannelSelector.java fba2dcb > flume-ng-core/src/main/java/org/apache/flume/Context.java 5294e31 > flume-ng-core/src/main/java/org/apache/flume/SinkProcessor.java d863ed0 > flume-ng-core/src/main/java/org/apache/flume/SinkProcessorType.java be1891b > > flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorFactory.java > 800f471 > > flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorType.java > 511fc65 > flume-ng-core/src/main/java/org/apache/flume/channel/ChannelType.java > d8419e8 > > flume-ng-core/src/main/java/org/apache/flume/channel/DefaultChannelFactory.java > 963a6a3 > > flume-ng-core/src/main/java/org/apache/flume/conf/ConfigurableComponent.java > PRE-CREATION > flume-ng-core/src/main/java/org/apache/flume/conf/Configurables.java > 84492e5 > flume-ng-core/src/main/java/org/apache/flume/sink/DefaultSinkFactory.java > b89dfa0 > flume-ng-core/src/main/java/org/apache/flume/sink/DefaultSinkProcessor.java > 257bab3 > flume-ng-core/src/main/java/org/apache/flume/sink/SinkGroup.java 0dffd69 > flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java > 6160a17 > flume-ng-core/src/main/java/org/apache/flume/sink/SinkType.java 6b08c09 > > flume-ng-core/src/main/java/org/apache/flume/source/DefaultSourceFactory.java > a610e6f > flume-ng-core/src/main/java/org/apache/flume/source/SourceType.java cd8991e > flume-ng-dist/pom.xml 642e681 > flume-ng-dist/src/main/assembly/dist.xml 7e401ae > flume-ng-dist/src/main/assembly/src.xml bd4f090 > > flume-ng-node/src/main/java/org/apache/flume/conf/properties/FlumeConfiguration.java > d66f6d1 > > flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java > 1f0e8c6 > pom.xml 3a9bc42 > > Diff: https://reviews.apache.org/r/4502/diff > > > Testing > ------- > > Functional testing done. > > > Thanks, > > Hari > >
