FLUME-1772: AbstractConfigurationProvider should remove component which throws exception from configure method.
Review Request #8421 - Created Dec. 7, 2012 and submitted
Catches Exception around all the configure() methods of components. If they throw an exception, they are removed from the configuration (not added).
Tests added for source, channel, and sink.
|If channels list is empty should we throw? I think we should just remove the source and log it right?||Hari Shreedharan||Dec. 18, 2012, 8:34 p.m.||Open|
|Same as above, should we throw or remove the component?||Hari Shreedharan||Dec. 18, 2012, 8:34 p.m.||Open|
|same as above||Hari Shreedharan||Dec. 18, 2012, 8:34 p.m.||Open|
Posted (Dec. 8, 2012, 12:39 a.m.)
Generally looks good. I have a couple of comments: * There are a lot of tabs/trailing spaces. Could you please remove those. * If we remove these components, we need to make sure that after all components are loaded/removed, we have at least 1 channel with a source or sink in the agent. Basically after all load* methods are done, we should iterate through the sinks and sources and verify their channels(in case of sources, at least one of the channels) are still around (if not the source/sink should be removed), else the config is invalid.
Posted (Dec. 18, 2012, 8:34 p.m.)
Posted (Dec. 19, 2012, 12:56 a.m.)
Posted (Dec. 20, 2012, 2:19 a.m.)
Brock, Generally looks good. I have a couple of questions though: * The components list in ChannelComponent class seems to be not used. The only time it is actually used is to check if it is empty (otherwise the components are simply added to it). I think this list can easily be replaced by a boolean right? * Assume that the components is required for something (which I missed), then we need 2 lists for sources and sinks, since we can have a source and a sink with the same name.
Review request changed
Updated (Dec. 20, 2012, 4:21 p.m.)
- changed from pending to submitted