[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
GitHub user stanlyDoge opened a pull request:

    https://github.com/apache/activemq-artemis/pull/1750

    ARTEMIS-1581 fix handshake-timeout property configurability

   

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/stanlyDoge/activemq-artemis E967

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/1750.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1750
   
----
commit 87136d4702b21fb48d14bbdb611995c49881f25d
Author: Stanislav Knot <sknot@...>
Date:   2018-01-04T16:35:11Z

    ARTEMIS-1581 fix handshake-timeout property configurability

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...

jgoodyear-2
Github user jbertram commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1750
 
    GitHub is reporting that this branch has conflicts.  Also it would be worth adding a test here to verify the fix since none of the existing tests caught it.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user stanlyDoge commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1750
 
    The build failed at MessageConsumerTest.testStopConnectionDuringOnMessage test, but it passes at local. Is that just coincidence it has failed? Can we re-run jenkins build?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1750
 
    @stanlyDoge could you squash the commits. Cheers.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user stanlyDoge commented on the issue:

    https://github.com/apache/activemq-artemis/pull/1750
 
    @michaelandrepearce Done
    @jbertram ahh, that explains my another conflict. Thanks.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1750#discussion_r159895861
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/server/config/JMSConfigurationTest.java ---
    @@ -97,4 +111,56 @@ public void testSetupJMSConfiguration() throws Exception {
     
           server.stop();
        }
    +
    +   @Test
    +   public void testHandshakeTimeoutWithValueSet() throws Exception {
    +      JMSServerManager jmsServer;
    --- End diff --
   
    The JMSServerManager is deprecated so it's probably best not to use it in new tests.  Check out the "embedded-simple" example for how to programmatically instantiate a broker with a configuration file.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1750#discussion_r159896349
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/server/config/JMSConfigurationTest.java ---
    @@ -97,4 +111,56 @@ public void testSetupJMSConfiguration() throws Exception {
     
           server.stop();
        }
    +
    +   @Test
    +   public void testHandshakeTimeoutWithValueSet() throws Exception {
    +      JMSServerManager jmsServer;
    --- End diff --
   
    Or don't even use a configuration file and just programmatically configure the acceptor.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user stanlyDoge commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1750#discussion_r160086429
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/server/config/JMSConfigurationTest.java ---
    @@ -97,4 +111,56 @@ public void testSetupJMSConfiguration() throws Exception {
     
           server.stop();
        }
    +
    +   @Test
    +   public void testHandshakeTimeoutWithValueSet() throws Exception {
    +      JMSServerManager jmsServer;
    --- End diff --
   
    Good point with JMSServerManager, thanks.
    The issue is about unability to set timeout value using conf file so using it is intentional. Setting this value programmatically is done [here](https://github.com/apache/activemq-artemis/pull/1534/files#diff-70ef2b6fc0b6f5e37ea6023acce65b8c). Or did you think anything else?


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/1750


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user jbertram commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1750#discussion_r160195533
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/server/config/JMSConfigurationTest.java ---
    @@ -97,4 +111,56 @@ public void testSetupJMSConfiguration() throws Exception {
     
           server.stop();
        }
    +
    +   @Test
    +   public void testHandshakeTimeoutWithValueSet() throws Exception {
    +      JMSServerManager jmsServer;
    --- End diff --
   
    The problematic use-case wasn't necessarily with a configuration file, but simply using a URL (which can be done programmatically).  I changed the test a bit before I merged it.  It's a bit simpler this way.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...

jgoodyear-2
In reply to this post by jgoodyear-2
Github user stanlyDoge commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1750#discussion_r160340487
 
    --- Diff: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/server/config/JMSConfigurationTest.java ---
    @@ -97,4 +111,56 @@ public void testSetupJMSConfiguration() throws Exception {
     
           server.stop();
        }
    +
    +   @Test
    +   public void testHandshakeTimeoutWithValueSet() throws Exception {
    +      JMSServerManager jmsServer;
    --- End diff --
   
    Aha. Thank you :)


---