> On 2012-03-02 16:47:32, Will McQueen wrote:
> > flume-ng-core/src/main/java/org/apache/flume/SinkProcessorType.java, line 40
> > <https://reviews.apache.org/r/4152/diff/2/?file=87333#file87333line40>
> >
> >     For consistency with the other types (SinkType, SourceType, ...), can 
> > we please put OTHER at the top of the enum listing?
> >     
> >     Also for same consistency, can we have the comment read:
> >     
> >       /**
> >        * Place holder for custom sink processors not part of this 
> > enumeration.
> >        */

Makes sense, done


> On 2012-03-02 16:47:32, Will McQueen wrote:
> > flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java,
> >  line 40
> > <https://reviews.apache.org/r/4152/diff/2/?file=87334#file87334line40>
> >
> >     Before this line:
> >     Preconditions.checkNotNull(context);
> >     Preconditions.checkNotNull(sinks);
> >     logger.debug("Creating instance of sink processor name {}, type {}", 
> > name, type);

Added precondition checks, changed the debug message a bit because processors 
belong to a sink group and thus themselves are not named.


> On 2012-03-02 16:47:32, Will McQueen wrote:
> > flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java,
> >  line 48
> > <https://reviews.apache.org/r/4152/diff/2/?file=87334#file87334line48>
> >
> >     => "Sink processor type {} is a custom type"
> >     
> >     (this string output matches the convention used by DefaultSourceFactory)

ok


> On 2012-03-02 16:47:32, Will McQueen wrote:
> > flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java,
> >  line 66
> > <https://reviews.apache.org/r/4152/diff/2/?file=87334#file87334line66>
> >
> >     add "sink", remove comma:
> >     
> >     =>"Unable to create sink processor type: "
> >

done


- Juhani


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


On 2012-03-02 07:51:49, Juhani Connolly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4152/
> -----------------------------------------------------------
> 
> (Updated 2012-03-02 07:51:49)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> Made custom processors possible in the same way as sink and source: the type 
> can represent a classname or a shorthand name from the typedef.
> 
> 
> This addresses bug FLUME-1001.
>     https://issues.apache.org/jira/browse/FLUME-1001
> 
> 
> Diffs
> -----
> 
>   flume-ng-core/src/main/java/org/apache/flume/SinkProcessorType.java be1891b 
>   flume-ng-core/src/main/java/org/apache/flume/sink/SinkProcessorFactory.java 
> 10f9f4e 
>   
> flume-ng-core/src/test/java/org/apache/flume/sink/TestSinkProcessorFactory.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/4152/diff
> 
> 
> Testing
> -------
> 
> Existing tests pass, except I'm having an issue with TestNetcatSource which 
> seems to be totally unrelated... It turns up on flume-728 head too, probably 
> something in my environment
> 
> Added a new test to verify that processors created by shorthand name and by 
> full class create the same class, verifying that creation by classname also 
> works
> 
> 
> Thanks,
> 
> Juhani
> 
>

Reply via email to