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

Ship it!


Couple of nits and a potential race on shutdown (which would, in the future, be 
used for reconfiguration). Fix those and +1.


flume-ng-legacy-sources/flume-avro-source/src/main/java/org/apache/flume/source/avroOG/AvroOGSource.java
<https://reviews.apache.org/r/3701/#comment10577>

    Unless this is for class name level compatibility, the package should be 
org.apache.flume...



flume-ng-legacy-sources/flume-thrift-source/pom.xml
<https://reviews.apache.org/r/3701/#comment10578>

    Push the version up to the parent pom's dependencyManagement section so we 
have centralized control over versions.



flume-ng-legacy-sources/flume-thrift-source/src/main/java/org/apache/flume/source/thrift/ThriftCompatibilitySource.java
<https://reviews.apache.org/r/3701/#comment10579>

    Join with threadHandlerThread so we now resources are released. If you 
receive an interrupt, propagate.



flume-ng-legacy-sources/flume-thrift-source/src/test/java/org/apache/flume/source/thriftOG/TestThriftOGSource.java
<https://reviews.apache.org/r/3701/#comment10580>

    Please don't use wildcard imports.



flume-ng-legacy-sources/pom.xml
<https://reviews.apache.org/r/3701/#comment10576>

    Probably shouldn't be called Flume NG Sources anymore, right?


- Eric


On 2012-02-02 17:57:05, Prasad Mujumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3701/
> -----------------------------------------------------------
> 
> (Updated 2012-02-02 17:57:05)
> 
> 
> Review request for Flume and Eric Sammer.
> 
> 
> Summary
> -------
> 
> Added thrift and sources to process flume-og thrift event formats.
> Flume OG clients can send events directly to these NG sources. Also Flume OG 
> agents can configure the rcpSinks to send events to these NG sinks.
> 
> 
> This addresses bug FLUME-942.
>     https://issues.apache.org/jira/browse/FLUME-942
> 
> 
> Diffs
> -----
> 
>   flume-ng-legacy-sources/flume-avro-source/pom.xml PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-avro-source/src/main/avro/flumeCompatibility.avdl
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-avro-source/src/main/java/org/apache/flume/source/avroOG/AvroOGSource.java
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-avro-source/src/test/java/org/apache/flume/source/avroOG/TestOGAvroSource.java
>  PRE-CREATION 
>   flume-ng-legacy-sources/flume-thrift-source/pom.xml PRE-CREATION 
>   flume-ng-legacy-sources/flume-thrift-source/src/main/java/README 
> PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/EventStatus.java
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/Priority.java
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/ThriftFlumeEvent.java
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/main/java/com/cloudera/flume/handlers/thrift/ThriftFlumeEventServer.java
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/main/java/org/apache/flume/source/thrift/ThriftCompatibilitySource.java
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/main/thrift/flumeCompatibility.thrift
>  PRE-CREATION 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/test/java/org/apache/flume/source/thriftOG/TestThriftOGSource.java
>  PRE-CREATION 
>   flume-ng-legacy-sources/pom.xml PRE-CREATION 
>   pom.xml 2bf32bb 
> 
> Diff: https://reviews.apache.org/r/3701/diff
> 
> 
> Testing
> -------
> 
> Regression tests
> Added tests for the new sinks
> Manually tested event transfer from OG agents to NG thrift and avro sources. 
> 
> 
> Thanks,
> 
> Prasad
> 
>

Reply via email to