----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4115/#review6199 -----------------------------------------------------------
Thanks for the patch Hari. The patch needs to be rebased on the latest sources and seems to have accidentally overlaid a lot of older code on top of the latest. For example, all the pom.xml files have their versions overwritten to 1.1.0-incubating. Not only the patch is very big, it actually has substantial formatting changes which are making it very hard to keep an eye on the actual change. Therefore, for the benefit of the reviewers and for minimizing the risk, I suggest you update the patch to only have the configuration subsystem changes. This means that all components will get configured like custom components that do not have stubs exposed. Once we get that part reviewed and checked in, we can revive the rest of the changes component by component and get them committed. I understand that you have gone through many painstaking revisions of the patch so far, and if you you so prefer - I will be happy to help out in breaking this patch into phased changes. - Arvind On 2012-03-19 23:01:52, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4115/ > ----------------------------------------------------------- > > (Updated 2012-03-19 23:01:52) > > > Review request for Flume. > > > Summary > ------- > > Currently the configuration of each component sits deep inside the component > themselves. There is no way to verify if a configuration is valid before run > time. In most systems like Flume, it is likely that these confs will be > deployed from one single host, on to the machines where flume agents are > running. Only when each agent starts, invalid confs are identified because > the Agents would terminate by throwing exceptions. This is a first attempt to > make a component-aware configuration system which is independent of the > Flume, and does not require the Flume jar to be installed. Each component > needs a configuration manager, which configures the components. > > I have not completed the validation part yet, but have completed the > configuration managers for all the components that come in the flume jar. I > will add support for the other components, such as HDFS sink, JDBC channel > etc soon. I will also add the validation support to the classes, but would > like a review of the model being proposed here. > > Note that I do know of a couple of bugs in the existing diff(like that if the > name of a config key starts with an Upper case letter, an exception is > thrown) - I will fix and upload the review here soon. > > > This addresses bug FLUME-992. > https://issues.apache.org/jira/browse/FLUME-992 > > > Diffs > ----- > > flume-ng-channels/flume-file-channel/pom.xml 48d1481 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/FileChannel.java > a279453 > flume-ng-channels/flume-jdbc-channel/pom.xml 8e82583 > > flume-ng-channels/flume-jdbc-channel/src/main/java/org/apache/flume/channel/jdbc/JdbcChannel.java > bca0c50 > > flume-ng-channels/flume-jdbc-channel/src/main/java/org/apache/flume/channel/jdbc/JdbcChannelProvider.java > e445d61 > > flume-ng-channels/flume-jdbc-channel/src/main/java/org/apache/flume/channel/jdbc/JdbcChannelProviderFactory.java > 6fbd6ef > > flume-ng-channels/flume-jdbc-channel/src/main/java/org/apache/flume/channel/jdbc/impl/JdbcChannelProviderImpl.java > 307ae89 > > flume-ng-channels/flume-jdbc-channel/src/test/java/org/apache/flume/channel/jdbc/TestJdbcChannelProvider.java > 7710d46 > flume-ng-channels/pom.xml 0bd8633 > flume-ng-clients/flume-ng-log4jappender/pom.xml d1c443d > > flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java > 68d95fb > flume-ng-clients/pom.xml 0a0dc04 > flume-ng-configuration/pom.xml 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/Context.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/channel/JdbcChannelConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/MemoryChannelConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/MultiplexingChannelSelectorConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/channel/PseudoTxnMemoryChannelConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/AvroSinkConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/FailoverSinkProcessorConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/HDFSSinkConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/RollingFileSinkConfiguration.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/AvroSourceConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/source/ExecSourceConfiguration.java > PRE-CREATION > > flume-ng-configuration/src/main/java/org/apache/flume/conf/source/NetcatSourceConfiguration.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-configuration/src/test/resources/flume-ng-error-test PRE-CREATION > flume-ng-configuration/src/test/resources/flume-ng-test.conf 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 ea6000b > flume-ng-core/src/main/java/org/apache/flume/SinkProcessorType.java be1891b > flume-ng-core/src/main/java/org/apache/flume/channel/AbstractChannel.java > 352bf08 > > 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/channel/MemoryChannel.java > bfa1fde > > flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java > 83928b7 > > flume-ng-core/src/main/java/org/apache/flume/channel/PseudoTxnMemoryChannel.java > 489d3e5 > > flume-ng-core/src/main/java/org/apache/flume/channel/ReplicatingChannelSelector.java > 8f22746 > flume-ng-core/src/main/java/org/apache/flume/conf/Configurable.java 0fa4839 > > 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/AvroSink.java 3edc563 > 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/FailoverSinkProcessor.java > 9f5b856 > flume-ng-core/src/main/java/org/apache/flume/sink/RollingFileSink.java > 7f1d3b3 > 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/AvroSource.java 903889d > > flume-ng-core/src/main/java/org/apache/flume/source/DefaultSourceFactory.java > a610e6f > flume-ng-core/src/main/java/org/apache/flume/source/ExecSource.java b6b1181 > flume-ng-core/src/main/java/org/apache/flume/source/NetcatSource.java > 94245ac > > flume-ng-core/src/main/java/org/apache/flume/source/SequenceGeneratorSource.java > 440c5a9 > flume-ng-core/src/main/java/org/apache/flume/source/SourceType.java 9082470 > flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java > d78d27f > flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java > 34818f0 > flume-ng-core/src/test/java/org/apache/flume/TestContext.java 51c350f > > flume-ng-core/src/test/java/org/apache/flume/channel/AbstractBasicChannelSemanticsTest.java > 6e71e46 > flume-ng-core/src/test/java/org/apache/flume/channel/MockChannel.java > 24b01e2 > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java > e070864 > > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelConcurrency.java > 8dad0b2 > > flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelTransaction.java > bc81f26 > flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 3765924 > > flume-ng-core/src/test/java/org/apache/flume/sink/TestFailoverSinkProcessor.java > 195c121 > flume-ng-core/src/test/java/org/apache/flume/sink/TestLoggerSink.java > 92ff6fe > flume-ng-core/src/test/java/org/apache/flume/sink/TestRollingFileSink.java > 7e26e2a > flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java > c5c3f2f > flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java > 6035270 > > flume-ng-core/src/test/java/org/apache/flume/source/TestPollableSourceRunner.java > c27f82c > > flume-ng-core/src/test/java/org/apache/flume/source/TestSequenceGeneratorSource.java > 579b257 > > flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java > d1db49d > flume-ng-dist/pom.xml f7e0dd9 > flume-ng-legacy-sources/flume-avro-source/pom.xml 216e479 > > flume-ng-legacy-sources/flume-avro-source/src/main/java/org/apache/flume/source/avroLegacy/AvroLegacySource.java > dde8f28 > > flume-ng-legacy-sources/flume-avro-source/src/test/java/org/apache/flume/source/avroLegacy/TestLegacyAvroSource.java > 6e3eb53 > flume-ng-legacy-sources/flume-thrift-source/pom.xml 70b67a6 > > flume-ng-legacy-sources/flume-thrift-source/src/main/java/org/apache/flume/source/thriftLegacy/ThriftLegacySource.java > fbf7362 > > flume-ng-legacy-sources/flume-thrift-source/src/test/java/org/apache/flume/source/thriftLegacy/TestThriftLegacySource.java > ddd9478 > flume-ng-legacy-sources/pom.xml 2e5b8dc > flume-ng-node/pom.xml ffa89fa > > 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 > 97f72e1 > > flume-ng-node/src/test/java/org/apache/flume/node/TestAbstractLogicalNodeManager.java > 521b586 > flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java > fb2a960 > flume-ng-node/src/test/resources/flume-conf.properties 848caca > flume-ng-sdk/pom.xml d1c6adf > flume-ng-sinks/flume-hdfs-sink/pom.xml 76d69a3 > > flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/HDFSEventSink.java > da82f7e > > flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestHDFSEventSink.java > 8fa72a1 > flume-ng-sinks/flume-irc-sink/pom.xml d935faf > > flume-ng-sinks/flume-irc-sink/src/main/java/org/apache/flume/sink/irc/IRCSink.java > 8e77218 > flume-ng-sinks/pom.xml acb3087 > pom.xml cf10727 > > Diff: https://reviews.apache.org/r/4115/diff > > > Testing > ------- > > All existing unit tests for the components whose configuration has now been > moved to these stubs pass. > > > Thanks, > > Hari > >
