[ 
https://issues.apache.org/jira/browse/FLUME-1181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13269036#comment-13269036
 ] 

[email protected] commented on FLUME-1181:
------------------------------------------------------



bq.  On 2012-05-05 10:51:23, Hari Shreedharan wrote:
bq.  > 
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java,
 line 129
bq.  > <https://reviews.apache.org/r/5017/diff/1/?file=106967#file106967line129>
bq.  >
bq.  >     Same as TCP source, we might want to handle NumberFormatExceptions 
for all getInteger calls. Maybe we should do that in Context itself.
bq.  
bq.  Brock Noland wrote:
bq.      I prefer throwing any NFE from context as it can silently hide 
problems where you thought the configuration was set appropriately. I think 
it's better to fail to start to let the user know things are not correct as 
opposed to continuing with a configuration they did not desire.

Brock - I was not suggesting that we not throw from Context. We could wrap them 
up and throw, but on the other hand it is better that the calling code would 
know exactly why it failed without using getCause(). Either way is fine, I 
suggested this because of the fact that every getInteger call seems to need an 
NFE catch clause, but if it isnt this is will still need a catch with 
FlumException. 


- Hari


-----------------------------------------------------------
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:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/5017/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-05-05 06:22:46)
bq.  
bq.  
bq.  Review request for Flume, Brock Noland and Hari Shreedharan.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  This change:
bq.  * introduces a precondition check in Context.getSubProperties(String 
prefix) method enforce that the prefix ends with a period,
bq.  * fixes the TestPropertiesFileConfigurationProvider to load the property 
file which was previously not working, and fixes cases that are broken
bq.  * refactors some of the source (not all) to externalize the configuration 
keys into separate constants class
bq.  
bq.  
bq.  This addresses bug FLUME-1181.
bq.      https://issues.apache.org/jira/browse/FLUME-1181
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/Context.java 
1334310 
bq.    
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/BasicConfigurationConstants.java
 PRE-CREATION 
bq.    
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/ComponentConfiguration.java
 1334310 
bq.    
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/FlumeConfiguration.java
 1334310 
bq.    
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/sink/SinkGroupConfiguration.java
 1334310 
bq.    
/trunk/flume-ng-configuration/src/main/java/org/apache/flume/conf/source/SourceConfiguration.java
 1334310 
bq.    
/trunk/flume-ng-core/src/main/java/org/apache/flume/channel/ChannelSelectorFactory.java
 1334310 
bq.    
/trunk/flume-ng-core/src/main/java/org/apache/flume/channel/MultiplexingChannelSelector.java
 1334310 
bq.    
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogSourceConfigurationConstants.java
 PRE-CREATION 
bq.    
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogTcpSource.java 
1334310 
bq.    
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUDPSource.java 
1334310 
bq.    
/trunk/flume-ng-core/src/main/java/org/apache/flume/source/SyslogUtils.java 
1334310 
bq.    
/trunk/flume-ng-node/src/main/java/org/apache/flume/conf/properties/PropertiesFileConfigurationProvider.java
 1334310 
bq.    
/trunk/flume-ng-node/src/test/java/org/apache/flume/conf/properties/TestPropertiesFileConfigurationProvider.java
 1334310 
bq.    /trunk/flume-ng-node/src/test/resources/flume-conf.properties 1334310 
bq.  
bq.  Diff: https://reviews.apache.org/r/5017/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  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.
bq.  
bq.  Also done some manual validation of the system with a few simple scenarios.
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Arvind
bq.  
bq.


                
> Context must enforce dot-separated prefix for sub-properties.
> -------------------------------------------------------------
>
>                 Key: FLUME-1181
>                 URL: https://issues.apache.org/jira/browse/FLUME-1181
>             Project: Flume
>          Issue Type: Bug
>          Components: Configuration
>            Reporter: Arvind Prabhakar
>            Assignee: Arvind Prabhakar
>             Fix For: v1.2.0
>
>         Attachments: FLUME-1181-1.patch
>
>
> Currently the method Context.getSubProperties(String prefix) assumes a 
> well-formed prefix. This can lead to undetected issues such as what is being 
> seen in the current state of channel selector configuration, where the keys 
> within the channel selector are being computed as prefixed with a period.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to