rabbitmq / rabbitmq-amqp1.0

AMQP 1.0 support for RabbitMQ
https://www.rabbitmq.com/
Other
93 stars 20 forks source link

Expand pattern in to_expiration to accept other types #14

Closed flaper87 closed 7 years ago

flaper87 commented 9 years ago

Fixes #13

michaelklishin commented 9 years ago

Thank you, I'd like to understand how we can reproduce #13 before evaluating the proposed fix.

flaper87 commented 9 years ago

@michaelklishin disclaimer, I've close to no knowledge about erlang so forgive me if that fix is completely wrong.

An code snippet that should help you reproduce this bug:

from proton import Message
from proton.handlers import MessagingHandler
from proton.reactor import Container

class HelloWorld(MessagingHandler):
    def __init__(self, server, address):
        super(HelloWorld, self).__init__()
        self.server = server
        self.address = address

    def on_start(self, event):
        conn = event.container.connect(self.server, reconnect=False)
        event.container.create_receiver(conn, self.address)
        event.container.create_sender(conn, self.address)

    def on_sendable(self, event):
        event.sender.send(Message(body=u"Hello World", ttl=500))
        event.sender.close()

    def on_message(self, event):
        print event.message.body
        event.connection.close()

Container(HelloWorld("127.0.0.1:5672", "examples")).run()

In other words, as soon as you set the ttl in the message, the plugin fails with the error on #13

Thanks

flaper87 commented 9 years ago

@michaelklishin Hi :)

Is the above snippet enough? Thanks for reviewing the patch

michaelklishin commented 9 years ago

Thank you, I think it should be enough — what client/version do you use?

We will get to this PR in the next few days.

flaper87 commented 9 years ago

Good point. Using python-qpid-proton should be enough.

pip install python-qpid-proton

Let me know if you run into troubles, I'll be happy to help.

kjnilsson commented 7 years ago

@flaper87 I have cherry-picked your commit to a new branch and created an updated PR with tests as well as ensuring full round-trip. See: #38

Closing this. Thank you for the repro and fix.