-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16151/#review56300
-----------------------------------------------------------



flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java
<https://reviews.apache.org/r/16151/#comment96613>

    Let's enhance the comment here. After a year nobody will remember what 
batchSize fix we needed :)



flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java
<https://reviews.apache.org/r/16151/#comment96614>

    Let's put the property names into quotes inside the string, e.g.:
    
    Property'batch-size' has been deprecated, please use 'batchSize' instead.
     
    I'm hoping that putting the names into quotes will make it more obvious 
that it's a config property and not just a text.



flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java
<https://reviews.apache.org/r/16151/#comment96615>

    Please don't use asterisk imports, always iterate what exactly you need to 
import.


- Jarek Cecho


On Dec. 10, 2013, 6:32 a.m., Ashish Paliwal wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16151/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2013, 6:32 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-2006
>     https://issues.apache.org/jira/browse/FLUME-2006
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> Patch based on discussion on JIRA
> 1. If batch-size is present, a warning is logged to to use batchSize
> 2. If both, batch-size and batchSize are present, an Exception is thrown, 
> recommending to use batchSize
> 3. batch-size property has been marked as deprecated
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/sink/AbstractRpcSink.java 
> 5146834 
>   flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 757a536 
>   flume-ng-sdk/pom.xml d5d50cf 
>   flume-ng-sdk/src/main/java/org/apache/flume/api/FailoverRpcClient.java 
> db6905a 
>   flume-ng-sdk/src/main/java/org/apache/flume/api/NettyAvroRpcClient.java 
> a2eb264 
>   
> flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientConfigurationConstants.java
>  136c504 
>   flume-ng-sdk/src/main/java/org/apache/flume/api/RpcClientFactory.java 
> 00d9cf0 
>   flume-ng-sdk/src/main/java/org/apache/flume/api/ThriftRpcClient.java 
> cf45ab9 
> 
> Diff: https://reviews.apache.org/r/16151/diff/
> 
> 
> Testing
> -------
> 
> Existing test cases which use batch-size pass
> Added test case to check behaviour if both properties are present
> 
> 
> Thanks,
> 
> Ashish Paliwal
> 
>

Reply via email to