You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by grs <gi...@git.apache.org> on 2016/08/10 08:05:34 UTC
[GitHub] activemq-artemis pull request #712: Artemis 447
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 <go...@gmail.com>
Date: 2016-08-09T19:53:09Z
Merge pull request #1 from apache/master
rebase from upstream
commit 5577e921bb8fb7c70718ae6e0afc2c91cf4aaed8
Author: Gordon Sim <gs...@redhat.com>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #712: Artemis 447
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the issue:
https://github.com/apache/activemq-artemis/pull/712
@grs I'm not sure what happened here, but it seems there's some commits from your local branch or something. Could you squash or resend with just the single commit please.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #712: Artemis 447
Posted by johnament <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #712: Artemis 447
Posted by mtaylor <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #712: Artemis 447
Posted by asfgit <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #712: Artemis 447
Posted by grs <gi...@git.apache.org>.
Github user grs commented on the issue:
https://github.com/apache/activemq-artemis/pull/712
@mtaylor sorry, I think that was because I merged rather than rebased when syncing my fork. Will avoid that next time.
@clebertsuconic thanks!
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #712: Artemis 447
Posted by johnament <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #712: Artemis 447
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/712
@mtaylor @grs The script we use to merge will fix it.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #712: Artemis 447
Posted by grs <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---