[GitHub] activemq-artemis pull request #712: Artemis 447

[grs]

Github user grs commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/712#discussion_r74246549
  
--- Diff: 
artemis-protocols/artemis-proton-plug/src/main/java/org/proton/plug/context/AbstractConnectionContext.java
 ---
@@ -73,6 +76,8 @@ public AbstractConnectionContext(AMQPConnectionCallback 
connectionCallback,
 ScheduledExecutorService 
scheduledPool) {
   this.connectionCallback = connectionCallback;
   this.containerId = (containerId != null) ? containerId : 
UUID.randomUUID().toString();
+  connectionProperties.put(Symbol.valueOf("product"), 
"apache-activemq-artemis");
--- End diff --

The version string is overrideable I believe so if you had some custom 
patched version you could indicate that. The brokers in Qpid don't make this 
value customisable, so I didn't personally think it was needed. I have no 
objection though if others have a use case for altering that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #712: Artemis 447

[mtaylor]

Github user mtaylor commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/712#discussion_r74240789
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/proton/ProtonTest.java
 ---
@@ -211,6 +213,25 @@ public void testBrokerContainerId() throws Exception {
}
 
@Test
+   public void testBrokerConnectionProperties() throws Exception {
+  if (protocol != 0 && protocol != 3) return; // Only run this test 
for AMQP protocol
--- End diff --

Yes.  It'd be better to use an AssumeTrue or similar.   We do this in many 
other areas of the test class, so perhaps we can update the to use the junit 
assertions in a separate commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #712: Artemis 447

[johnament]

Github user johnament commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/712#discussion_r74238129
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/proton/ProtonTest.java
 ---
@@ -211,6 +213,25 @@ public void testBrokerContainerId() throws Exception {
}
 
@Test
+   public void testBrokerConnectionProperties() throws Exception {
+  if (protocol != 0 && protocol != 3) return; // Only run this test 
for AMQP protocol
--- End diff --

Can this be an assume statement (this way the test will be marked as 
skipped if this is not true)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #712: Artemis 447

[asfgit]

Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #712: Artemis 447

[johnament]

Github user johnament commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/712#discussion_r74238033
  
--- Diff: 
artemis-protocols/artemis-proton-plug/src/main/java/org/proton/plug/context/AbstractConnectionContext.java
 ---
@@ -73,6 +76,8 @@ public AbstractConnectionContext(AMQPConnectionCallback 
connectionCallback,
 ScheduledExecutorService 
scheduledPool) {
   this.connectionCallback = connectionCallback;
   this.containerId = (containerId != null) ? containerId : 
UUID.randomUUID().toString();
+  connectionProperties.put(Symbol.valueOf("product"), 
"apache-activemq-artemis");
--- End diff --

Should this be overrideable in some way?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #712: Artemis 447

[grs]

GitHub user grs opened a pull request:

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

Artemis 447



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

$ git pull https://github.com/grs/activemq-artemis ARTEMIS-447

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

https://github.com/apache/activemq-artemis/pull/712.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 #712


commit 54c52a0274ee014a249d6d7d407de307477d0c9d
Author: grs 
Date:   2016-08-09T19:53:09Z

Merge pull request #1 from apache/master

rebase from upstream

commit 5577e921bb8fb7c70718ae6e0afc2c91cf4aaed8
Author: Gordon Sim 
Date:   2016-08-09T20:41:48Z

ARTEMIS-447: add name and version to properties in amqp open frame




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---