----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8369/#review14411 -----------------------------------------------------------
flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/DefaultJMSMessageConverter.java <https://reviews.apache.org/r/8369/#comment30662> Should be ObjectMessage flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/DefaultJMSMessageConverter.java <https://reviews.apache.org/r/8369/#comment30663> I think we should make the charset configurable. flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/DefaultJMSMessageConverter.java <https://reviews.apache.org/r/8369/#comment30664> We should wrap the exception in a FlumeException and throw. flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSMessageConsumer.java <https://reviews.apache.org/r/8369/#comment30673> pollTime should be called pollTimeout - it wasn't evident what this is until I actually realized it was the timeout value. flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSMessageConsumer.java <https://reviews.apache.org/r/8369/#comment30671> Since messageSelector is Nullable, this can throw NPE. Should the annotation be @NotNull? flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSMessageConsumerFactory.java <https://reviews.apache.org/r/8369/#comment30676> pollTime -> pollTimeout - Hari Shreedharan On Dec. 12, 2012, 10:13 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8369/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2012, 10:13 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. > 4) Adds a few abstract classes to core. The purpose of these was to clarify > the semantics of the configure, start, stop methods. > 5) Adds one protected method to AbtractSource to allow subclasses to set the > state. This is used to set the error state when an exception is thrown during > start, configure, or stop. > > > 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 > flume-ng-sources/pom.xml 48f751d > pom.xml 6b465b2 > > Diff: https://reviews.apache.org/r/8369/diff/ > > > Testing > ------- > > The tests which were added, pass. > > > Thanks, > > Brock Noland > >
