search

majek / puka

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

Passive flag for queue_declare #51

Closed ov7a closed 10 years ago

ov7a commented 10 years ago

I've encountered a case where I should monitor queue size. Because passive queue declaration is hardcoded as false here, I've added a new method.

P.S. This is my first pull-request, so please, be merciful :)

majek commented 10 years ago

Hmm. I think queue_size is given as a field on the promise returned by queue_declare...

majek commented 10 years ago

Ah, I see. Passive queue declaration is false.... Why you want to use passive=True? What's the benefit?

ov7a commented 10 years ago

As documentation says:

bit passive If set, the server will reply with Declare-Ok if the exchange already exists with the same name, and raise an error if not. The client can use this to check whether an exchange exists without modifying the server state. When set, all other method fields except name and no-wait are ignored. A declare with both passive and no-wait has no effect. Arguments are compared for semantic equivalence. If set, and the exchange does not already exist, the server MUST raise a channel exception with reply code 404 (not found). If not set and the exchange exists, the server MUST check that the existing exchange has the same values for type, durable, and arguments fields. The server MUST respond with Declare-Ok if the requested exchange matches these fields, and MUST raise a channel exception if not.

So, to check size of some queue one shouldn't know all its parameters and size checking will have no side effects.

majek commented 10 years ago

Understood. I forced passive=False as I didn't know if it's useful at all. You convinced me it can be useful. I'm okay with adding a passive=False default parameter to queue_declare. Would that work for you?

ov7a commented 10 years ago

Yep, I think so.

majek commented 10 years ago

Fancy updating your patch?

On Thu, Feb 13, 2014 at 1:10 PM, ov7a notifications@github.com wrote:

Yep, I think so.

— Reply to this email directly or view it on GitHubhttps://github.com/majek/puka/pull/51#issuecomment-34976157 .

ov7a commented 10 years ago

I can make another pull request with passive=False default parameter to queue_declare, if i understood you correctly.

majek commented 10 years ago

yup. please do. You can update this pull request (by using git push -f), but submitting a new pull request will work as well.

On Thu, Feb 13, 2014 at 1:13 PM, ov7a notifications@github.com wrote:

I can make another pull request with passive=False default parameter to queue_declare, if i understood you correctly.

— Reply to this email directly or view it on GitHubhttps://github.com/majek/puka/pull/51#issuecomment-34976381 .

ov7a commented 10 years ago

Something like this? Is everything correct?