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.
>
> Hari Shreedharan wrote:
>     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).

I updated the patch, please see the JMSSource configure() method.
- Brock
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/8369/#review14462
-----------------------------------------------------------
On Dec. 13, 2012, 11:47 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, 11:47 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