> On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java, > > line 399 > > <https://reviews.apache.org/r/4502/diff/1/?file=96766#file96766line399> > > > > Need to handle the case where the config is not specified, but the > > component is a standard component. In such cases, the component type enum > > should be used to determine the config holder class.
If it is a known component, the getKnownChannel function returns the ChannelType enum instance which points to that channel, and that is set to the config object. This config object is what is used to create the configuration object. If we have memory in the conf, chType will be ChannelType.MEMORY and toString() will return MEMORY. This is then used to create the MemoryChannelConfiguration object. Therefore, standard components are handled, so the case mentioned above works ok. > On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java, > > line 312 > > <https://reviews.apache.org/r/4502/diff/1/?file=96766#file96766line312> > > > > Looks like this check needs to happen before the if(channelSet.size() > > == 0) check. Yes, will move it up. > On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java, > > lines 300-301 > > <https://reviews.apache.org/r/4502/diff/1/?file=96766#file96766line300> > > > > I see that this check has been disabled for various components. Without > > this, the active component check, required attribute checks will not > > happen. How is the code handling those right now? Each of the basic configuration types - SourceConfiguration, SinkConfiguration, ChannelConfiguration etc. do the basic checks required for individual components. > On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java, > > lines 88-90 > > <https://reviews.apache.org/r/4502/diff/1/?file=96766#file96766line88> > > > > This seems redundant as the addRawProperty() method will perform these > > checks anyway. I believe this is done in FlumeConfiguration even now, I will remove it anyway. > On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java, > > lines 36-42 > > <https://reviews.apache.org/r/4502/diff/1/?file=96766#file96766line36> > > > > These dependencies are missing. You need to move them into this module. These are in a different patch, I will add them here. > On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/Context.java, > > line 20 > > <https://reviews.apache.org/r/4502/diff/1/?file=96765#file96765line20> > > > > Please do not change the package as that will break backward > > compatibility. Yes.will do. > On 2012-03-27 17:31:44, Arvind Prabhakar wrote: > > flume-ng-configuration/src/main/java/org/apache/flume/conf/ConfigurationException.java, > > line 21 > > <https://reviews.apache.org/r/4502/diff/1/?file=96764#file96764line21> > > > > This should extend FlumeException to be consistent. ok - Hari ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4502/#review6429 ----------------------------------------------------------- On 2012-03-27 06:17:30, Hari Shreedharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/4502/ > ----------------------------------------------------------- > > (Updated 2012-03-27 06:17:30) > > > 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/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-core/pom.xml 37fb112 > flume-ng-core/src/main/java/org/apache/flume/Context.java 5294e31 > > 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/test/java/org/apache/flume/sink/TestFailoverSinkProcessor.java > 3358cf4 > 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-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 > 50b9f0c > pom.xml c91222f > > Diff: https://reviews.apache.org/r/4502/diff > > > Testing > ------- > > Functional testing done. > > > Thanks, > > Hari > >
