You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by stanlyDoge <gi...@git.apache.org> on 2018/01/04 16:36:00 UTC
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
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 <sk...@...>
Date: 2018-01-04T16:35:11Z
ARTEMIS-1581 fix handshake-timeout property configurability
----
---
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1750
---
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
Posted by jbertram <gi...@git.apache.org>.
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.
---
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
Posted by stanlyDoge <gi...@git.apache.org>.
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?
---
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
Posted by jbertram <gi...@git.apache.org>.
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.
---
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
Posted by stanlyDoge <gi...@git.apache.org>.
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 :)
---
[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...
Posted by stanlyDoge <gi...@git.apache.org>.
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.
---
[GitHub] activemq-artemis pull request #1750: ARTEMIS-1581 fix handshake-timeout prop...
Posted by jbertram <gi...@git.apache.org>.
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.
---
[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...
Posted by stanlyDoge <gi...@git.apache.org>.
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?
---
[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1750
@stanlyDoge could you squash the commits. Cheers.
---
[GitHub] activemq-artemis issue #1750: ARTEMIS-1581 fix handshake-timeout property co...
Posted by jbertram <gi...@git.apache.org>.
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.
---