search

tjmehta / coworkers

A RabbitMQ Microservice Framework in Node.js
MIT License
609 stars 36 forks source link

Added ability to pass prefetch count in as a consumeOpts during connect #39

Closed sappleg closed 8 years ago

sappleg commented 8 years ago

Adds ability to specify a prefetch count on a consumer channel in accordance to amqplib's documentation. Requested in #38

The implementation uses the existing infrastructure around passing consumer options to the consumer channel during connection. It will check for the presence of a prefetch key in the consumeOpts object and call the prefetch method on the consumerChannel while passing in the provided value for prefetch in the consumeOpts object. It will then delete the prefetch key from the consumeOpts object if it exists as to not change how the system works down the line.

I could see how this could be viewed as hacky. @tjmehta, let me know if you had a cleaner implementation in mind to get the prefetch count to the assertAndConsumeAppQueue method.

sappleg commented 8 years ago

@tjmehta can't seem to get line 21 of assert-and-consume-app-queue.js covered by specs. Any clues?

tjmehta commented 8 years ago

Hey, nice I'm actually out of the office for a few hours but I can definitely wrap this up for you when I get back

tjmehta commented 8 years ago

I also think this may be better to add as an additional option to connect. Since queue is already taking quite a by of options. But let me know what you think

sappleg commented 8 years ago

@tjmehta Is this more along the lines of what you are thinking? If so, I need to make sure connect can correctly take the optional prefetch argument

tjmehta commented 8 years ago

Im checking this out now

tjmehta commented 8 years ago

Hey I was going to branch off this PR, but I ended up implementing a brand new method to handle this app.prefetch(...). I think this way is cleaner than overloading the existing methods, sorry about that. So I am going to close this. PR #41