Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Flume >> mail # dev >> Review Request: FLUME-924: Implement a JMS source for Flume NG


Copy link to this message
-
Re: Review Request: FLUME-924: Implement a JMS source for Flume NG


> On Dec. 13, 2012, 8:58 p.m., Hari Shreedharan wrote:
> > Mostly looks good, except the one comment I have.
>
> Brock Noland wrote:
>     Hi,
>    
>     I'd like to keep the Builder interface simply because it's how the rest of the components work. Once we settle on a new Interface this one can be changed to align with them.
>    
>     Brock
>
> Hari Shreedharan wrote:
>     Which components? The HTTPSource does not - but others do. I don't see the point of a builder interface here - since it does not add any benefit (I don't see it in others also, but they are backwards incompatible to remove. I really really don't want to add something that is backwards incompatible, yet we do have a cleaner way of implementing it).
>
> Brock Noland wrote:
>     The other components I was referring to is Serializers, File Channel Encryption, and Interceptors. I do feel it is adding value in this case. By using this paradigm the members of the JMSMessageConverter implementors can be Immutable where as they cannot be via the Configurable interface.

Brock,

How about this? Make the Builder optional. Check if the builder exists - if yes, use it, else call newInstance(). Also the config should just have the class name of the converter and not require the Builder name (the $Builder stuff being required should not be the case).
- Hari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8369/#review14462
-----------------------------------------------------------
On Dec. 13, 2012, 3:42 p.m., Brock Noland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8369/
> -----------------------------------------------------------
>
> (Updated Dec. 13, 2012, 3:42 p.m.)
>
>
> Review request for Flume.
>
>
> Description
> -------
>
> Implements FLUME-924 (JMS Source). Some notes:
>
> 1) Should work for any JMS provider but only tested with ActiveMQ.
> 2) Has unit tests as well as an ActiveMQ integration test
> 3) Allows for a pluggable class to convert a JMS message to a Flume Event. With that said, it provides a sensible default implementation.
>
>
> This addresses bug FLUME-924.
>     https://issues.apache.org/jira/browse/FLUME-924
>
>
> Diffs
> -----
>
>   flume-ng-doc/sphinx/FlumeUserGuide.rst 265f546
>   flume-ng-sources/flume-jms-source/pom.xml PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/DefaultJMSMessageConverter.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/InitialContextFactory.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSDestinationType.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSMessageConsumer.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSMessageConsumerFactory.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSMessageConverter.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSource.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSourceConfiguration.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/JMSMessageConsumerTestBase.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestDefaultJMSMessageConverter.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestIntegrationActiveMQ.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestJMSMessageConsumer.java PRE-CREATION
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestJMSSource.java PRE-CREATION