You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Jarek Cecho <ja...@apache.org> on 2014/10/11 22:19:22 UTC
Re: Review Request 16151: Patch for FLUME-2006 - in Avro the batch
size is called batch-size, in all other sources batchSize
-----------------------------------------------------------
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
>
>