You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by calohmn <gi...@git.apache.org> on 2018/08/21 09:08:34 UTC
[GitHub] activemq-artemis pull request #2256: ARTEMIS-2045 Add support for setting de...
GitHub user calohmn opened a pull request:
https://github.com/apache/activemq-artemis/pull/2256
ARTEMIS-2045 Add support for setting delivery annotations on AMQPMessage
This fixes ARTEMIS-2045.
It enables outgoing AMQP interceptors to set delivery annotations for the outgoing message.
A getter for delivery annotations on incoming messages (useful for incoming AMQP interceptors) has been added as well.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bsinno/activemq-artemis PR/amqp_outgoing_deliveryannotations
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2256.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 #2256
----
commit 0dcbb860f61a64e48893c016cd257f93f2203ca2
Author: Carsten Lohmann <ca...@...>
Date: 2018-08-21T09:00:01Z
ARTEMIS-2045 Add support for setting delivery annotations on outgoing message
----
---
[GitHub] activemq-artemis issue #2256: ARTEMIS-2045 Add support for setting delivery ...
Posted by tabish121 <gi...@git.apache.org>.
Github user tabish121 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2256
@jbertram yes, the broker can add additional message annotations as long as those being added follow the rules from the spec, if the user adds something that the remote doesn't understand and it is not prefixed with "x-opt" the remote can just kill the link.
http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-annotations
---
[GitHub] activemq-artemis pull request #2256: ARTEMIS-2045 Add support for setting de...
Posted by calohmn <gi...@git.apache.org>.
Github user calohmn commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2256#discussion_r211574987
--- Diff: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java ---
@@ -733,6 +743,9 @@ public void sendBuffer(ByteBuf buffer, int deliveryCount) {
buffer.writeBytes(data.duplicate().limit(headerEnds).byteBuffer());
}
+ writeOutgoingDeliveryAnnotations(buffer);
--- End diff --
I'm not sure here:
This addition here (in `sendBuffer(buffer, deliveryCount)` from the `Message` interface) was done to have both the `sendBuffer` and `getSendBuffer` methods behave the same towards using the `outgoingDeliveryAnnotations`.
But on the other hand this `sendBuffer` method isn't called by the `ProtonServerSenderContext` (only `getSendBuffer` is).
So maybe the better solution would be to remove usage of `outgoingDeliveryAnnotations` in the `sendBuffer` method here and rename `getSendBuffer` to `getOutgoingMessageSendBuffer` to avoid confusion concerning these methods.
---
[GitHub] activemq-artemis issue #2256: ARTEMIS-2045 Add support for setting delivery ...
Posted by calohmn <gi...@git.apache.org>.
Github user calohmn commented on the issue:
https://github.com/apache/activemq-artemis/pull/2256
Any comments on this?
For the case that the newly introduced `outgoingDeliveryAnnotations` have not been set, the code is still the same as before. I only have adapted some comments and a method name.
---
[GitHub] activemq-artemis issue #2256: ARTEMIS-2045 Add support for setting delivery ...
Posted by calohmn <gi...@git.apache.org>.
Github user calohmn commented on the issue:
https://github.com/apache/activemq-artemis/pull/2256
Rebased and updated after the refactorings done in #2329.
---
[GitHub] activemq-artemis issue #2256: ARTEMIS-2045 Add support for setting delivery ...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/2256
@tabish121, is it OK by the AMQP 1.0 spec for the broker to add delivery annotations?
---
[GitHub] activemq-artemis issue #2256: ARTEMIS-2045 Add support for setting delivery ...
Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the issue:
https://github.com/apache/activemq-artemis/pull/2256
Delivery annotations are for per-hop usage, so the brokers the only thing that should be adding delivery annotations to messages it is sending.
---
[GitHub] activemq-artemis pull request #2256: ARTEMIS-2045 Add support for setting de...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2256
---