You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Attila Simon <sa...@cloudera.com> on 2016/10/27 13:37:46 UTC

Re: Review Request 51802: FLUME-2976: Exception when JMS source tries to connect to a Weblogic server without authentication

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




flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSource.java (line 127)
<https://reviews.apache.org/r/51802/#comment223529>

    Non breaking change as the password can still be specified as "". (eg by creating an empty password file with touch and specifying that in flume conf)



flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestIntegrationActiveMQ.java (lines 87 - 102)
<https://reviews.apache.org/r/51802/#comment222556>

    Could you please use an Enum instead of boolean flags?


- Attila Simon


On Sept. 12, 2016, 1:08 p.m., Denes Arvay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51802/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2016, 1:08 p.m.)
> 
> 
> Review request for Flume, Bal�zs Don�t Bessenyei, Mike Percy, and Attila Simon.
> 
> 
> Bugs: FLUME-2976
>     https://issues.apache.org/jira/browse/FLUME-2976
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> If no `userName` and `passwordFile` is set for the JMS source it sets the password to `Optional("")`. This leads to an exception in the weblogic jndi context implementation when trying to connect to a weblogic jms server.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSource.java 7631827 
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestIntegrationActiveMQ.java 53cc09a 
> 
> Diff: https://reviews.apache.org/r/51802/diff/
> 
> 
> Testing
> -------
> 
> - `flume-ng-sources/flume-jms-source` tests pass
> - `TestIntegrationActiveMQ` is now a parameterized test to test with and without authentication.
> 
> 
> Thanks,
> 
> Denes Arvay
> 
>


Re: Review Request 51802: FLUME-2976: Exception when JMS source tries to connect to a Weblogic server without authentication

Posted by Attila Simon <sa...@cloudera.com>.

> On Oct. 27, 2016, 1:37 p.m., Attila Simon wrote:
> > flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSource.java, line 127
> > <https://reviews.apache.org/r/51802/diff/1/?file=1496685#file1496685line127>
> >
> >     Non breaking change as the password can still be specified as "". (eg by creating an empty password file with touch and specifying that in flume conf)

So the change itself looks good to me.


- Attila


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


On Sept. 12, 2016, 1:08 p.m., Denes Arvay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51802/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2016, 1:08 p.m.)
> 
> 
> Review request for Flume, Bal�zs Don�t Bessenyei, Mike Percy, and Attila Simon.
> 
> 
> Bugs: FLUME-2976
>     https://issues.apache.org/jira/browse/FLUME-2976
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> If no `userName` and `passwordFile` is set for the JMS source it sets the password to `Optional("")`. This leads to an exception in the weblogic jndi context implementation when trying to connect to a weblogic jms server.
> 
> 
> Diffs
> -----
> 
>   flume-ng-sources/flume-jms-source/src/main/java/org/apache/flume/source/jms/JMSSource.java 7631827 
>   flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestIntegrationActiveMQ.java 53cc09a 
> 
> Diff: https://reviews.apache.org/r/51802/diff/
> 
> 
> Testing
> -------
> 
> - `flume-ng-sources/flume-jms-source` tests pass
> - `TestIntegrationActiveMQ` is now a parameterized test to test with and without authentication.
> 
> 
> Thanks,
> 
> Denes Arvay
> 
>