Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support priority queue declarations #180

Merged
merged 1 commit into from
Sep 3, 2015
Merged

Support priority queue declarations #180

merged 1 commit into from
Sep 3, 2015

Conversation

ebardes
Copy link

@ebardes ebardes commented Aug 25, 2015

I'm working on project that benefits from using priority queues in RabbitMQ (available since version 3.5.x). I feel that others might benefit from this functionality too.

@michaelklishin
Copy link

@ebardes you already can use priority queues with any client that supports optional queue arguments. While I couldn't quickly find an example of how to do that with this client in either examples or tests, I'd be really surprised if amqp.node didn't provide a way to do that.

@ebardes
Copy link
Author

ebardes commented Aug 25, 2015

I couldn't find a way to declare a queue with the 'x-max-priority' flag. Perhaps I just missed it or that I'm not familiar enough with the nuances of amqp.node yet.

If you declare a queue through the management interface, the runtime code errors because the queue declarations don't match.

@michaelklishin
Copy link

Ah, so there's known argument filtering going on.

@ebardes
Copy link
Author

ebardes commented Aug 25, 2015

Exactly. I just added another known argument.

@nikosd23
Copy link

nikosd23 commented Sep 3, 2015

This is very important for us! When do you plan to perform a new release with this included?

@michaelklishin
Copy link

@squaremo this PR looks good to me.

@nikosd23
Copy link

nikosd23 commented Sep 3, 2015

Tested with RabbitMQ 3.3.4 broker and it seems to create the queues successfully, ignoring the x-max-priority option making the change backwards compatible

@squaremo
Copy link
Collaborator

squaremo commented Sep 3, 2015

Ah, so there's known argument filtering going on.

No there's not .. I see why you'd think that though. The second argument to #assertQueue is options, and an x-* argument would go in the arguments table in the options:

ch.assertQueue('prio', {arguments: {'x-max-priority': 10}});

whereas the "known options" go in the top level:

ch.assertQueue('prio', {maxPriority: 10});

Anyway, yes this PR looks fine.

squaremo added a commit that referenced this pull request Sep 3, 2015
Support priority queue declarations
@squaremo squaremo merged commit b22464c into amqp-node:master Sep 3, 2015
@michaelklishin
Copy link

@nikosd23 all unknown arguments will be ignored. They are all optional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants