You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by SreeramGarlapati <gi...@git.apache.org> on 2016/01/13 19:19:52 UTC

[GitHub] qpid-proton pull request: support one or more data sections in amq...

GitHub user SreeramGarlapati opened a pull request:

    https://github.com/apache/qpid-proton/pull/54

    support one or more data sections in amqpMessage

    This is a suggestion on how to implement the feature request PROTON-1098 and is NOT COMPLETE, although, the code works (while sending the message - I am able to send an AmqpMessage with Multiple Data sections using this implementation).
    
    Usage:
    While sending a Message, instead of creating the message with Data section, the following can be done:
    		Message amqpMessage = Proton.message();
    		amqpMessage.setBody(new DataList(dataList))
    
    (tagging the proton-j expert: @gemmellr to do the needful)

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

    $ git pull https://github.com/SreeramGarlapati/qpid-proton amqpMsg.supportOneOrMoreDataSections

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

    https://github.com/apache/qpid-proton/pull/54.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 #54
    
----
commit 76559ad506e1c26aed98cadd601ddb56899607fe
Author: Sreeram Garlapati <sr...@microsoft.com>
Date:   2016-01-13T18:05:45Z

    proton-j: Support one or more Data sections in AmqpMessage

commit b6de9ba02819492a802211d80ce0acd2e3f2f3cb
Author: Sreeram Garlapati <sr...@microsoft.com>
Date:   2016-01-13T18:06:15Z

    proton-j: Support one or more Data sections in AmqpMessage

----


---
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] qpid-proton pull request: PROTON-1098 : support one or more data s...

Posted by SreeramGarlapati <gi...@git.apache.org>.
Github user SreeramGarlapati commented on the pull request:

    https://github.com/apache/qpid-proton/pull/54#issuecomment-172112472
  
    Great Thanks a lot @gemmellr 
    & yes, I am adding the test on our end as well.
    
    Thanks!
    Sree


---
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] qpid-proton pull request: support one or more data sections in amq...

Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the pull request:

    https://github.com/apache/qpid-proton/pull/54#issuecomment-171666793
  
    Hi Sreeram
    
    Expanding the Message[Impl] to support multiple body sections is something that would be good to see done, but isn't something I've had time for previously and I'm working on other things at the moment. Whilst this change certainly works, I'm not sure the approach would fit in particularly well with making those more general improvements. 
    
    It is worth noting that the Message[Impl] in proton-j is basically just a helper in the end that you don't actually have to use, the actual message bytes sent via the Delivery object can be generated in other ways.
    
    In the mean time, with what is available now there are some ways I think you could probably send multiple Data sections currently, for example:
    - Use an Message object with any headers/delivery-annotations/message-annotations/properties/application-properties (and perhaps even the first data section set as the body), encode that.
    - Encode subsequent data body sections and append to the previous bytes.
    
    For the second bit above there would be various options, such as:
    - Use a Message object with only a data body set, encode that and append to previous bytes, repeating as needed for the n'th data section.
    - Use the very confusingly named org.apache.qpid.proton.codec.Data (which has nothing specifically to do with data sections) to write out all the data sections as DescribedType's. We do something similar in a test peer for the JMS client codebase, e.g at [1] it prepares an entire message this way with the body content possibly being a DataDescribedType [2].
    - Write a simple encoder to generate the data section bytes by writing out the necessary descriptor + Binary format/length bytes followed by the content bytes.
    
    [1] https://github.com/apache/qpid-jms/blob/2f3c1988606c215cca0bbb77d3462cdc15c36016/qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java#L1443
    [2] https://github.com/apache/qpid-jms/blob/2f3c1988606c215cca0bbb77d3462cdc15c36016/qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/describedtypes/sections/DataDescribedType.java
    Note that [2] is using the symbolic descriptor, but for non-test code the numeric descriptor would probably be preferred.
    



---
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] qpid-proton pull request: PROTON-1098 : support one or more data s...

Posted by SreeramGarlapati <gi...@git.apache.org>.
Github user SreeramGarlapati closed the pull request at:

    https://github.com/apache/qpid-proton/pull/54


---
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] qpid-proton pull request: PROTON-1098 : support one or more data s...

Posted by SreeramGarlapati <gi...@git.apache.org>.
Github user SreeramGarlapati commented on the pull request:

    https://github.com/apache/qpid-proton/pull/54#issuecomment-171718727
  
    Hello Robbie,
    
    Thanks a lot for taking time to elaborate the Options Robbie. That was really helpful. 
    Option (1) - using the MessageImpl utility to generate Multiple Data sections - seems very reasonable. 
    -  One ask here is to - have an explicit contract on MessageImpl.encode(...) to generate "no encoding" of any other sections when Only Data section is mentioned. It will be great if a Unit Test can be added to keep this behv. constant.
    
    Thanks again for great collaboration.!
    sree


---
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] qpid-proton pull request: PROTON-1098 : support one or more data s...

Posted by gemmellr <gi...@git.apache.org>.
Github user gemmellr commented on the pull request:

    https://github.com/apache/qpid-proton/pull/54#issuecomment-171998673
  
    The other sections don't exist on the message unless something is done to create them (they are all optional) so they wont be encoded in that case. I added a test showing that, which happens to do a simplistic variant of the third option in the process. I'd suggest that you probably want to add such a test on your side as well anyway.


---
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.
---