majek / puka

Puka - the opinionated RabbitMQ client
https://github.com/majek/puka
Other
182 stars 34 forks source link

None return value of "wait" method on timeout #47

Closed okomestudio closed 11 years ago

okomestudio commented 11 years ago

To illustrate the issue quickly, here's a snippet of code:

...
consume_promise = client.basic_consume('queue', prefetch_count=1)
message = client.wait(consume_promise, timeout=1.0)
...

The content of "message" would be None if there is nothing on the queue and the wait times out after 1 second. In effect this could be used to probe if the queue has been empty for the duration of the timeout.

The issue occurs when the "valid" messages passed around include None itself. In that case, I don't believe we have a robust way of distinguishing between a valid None message fetched from the queue and a None object returned due to timeout.

Currently, I do timing before and after the wait, and decide a None is due to empty queue if that timing is longer than the timeout. Wouldn't there be a better solution to indicate empty queue (like raising some exception, which Queue does for example).

I realize it probably is a bad design to pass around None as a message to begin with, but legacy code often cannot be touched effectively.

majek commented 11 years ago

client.wait doesn't return a "message". It returns a frame. You can access the body of the frame like this: message['body']. Or headers by message['headers'].

Additionally, a body of the message is a string, and can't be a python value "None".

I don't think there is a problem. Can you prepare a test code that would show the problem?

okomestudio commented 11 years ago

Okay, let me investigate this a bit more... there seems to be something really amiss here.

okomestudio commented 11 years ago

Hi there. So it looks like I was totally hallucinating. I don't know exactly why I was implementing something awkward as above (there usually are reasons for dubmness...), but I probably was confusing the issue with the behavior of a wrapper (written around puka) that I was having to deal with, and didn't rework the workflow when I started using puka directly. Sorry for the noise!!