-
Notifications
You must be signed in to change notification settings - Fork 69
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
Promise support? #14
Comments
No. It would increase the complexity of the code base and testing overhead. |
Could you expose classes via |
I'll create a branch and do it there. You happy to test / feedback? |
Thanks! It won't be better! |
Try
|
Thanks for your effort. However it doesn't fully work at least for broker (I haven't tried other classes yet) because:
I would try to make some changes and will send you PRs whose implementations are friendly to promisify. |
Thanks. I'm happy to accept a PR. |
I made first attempt to make Broker promisify friendly: Simple example rewritten in Promise way and ES 2017: import bluebird from 'bluebird'
import Rascal from 'rascal'
bluebird.promisifyAll(Rascal.Broker)
bluebird.promisifyAll(Rascal.Broker.prototype)
(async () => {
const broker = await Rascal.Broker.createAsync(Rascal.withDefaultConfig(config))
broker.on('error', console.error)
const subscription = await broker.subscribeAsync('demo_sub')
subscription
.on('message', function(message, content, ackOrNack) {
console.log(content)
ackOrNack()
})
.on('error', console.error)
setInterval(async () => {
await publication = broker.publishAsync('demo_pub', new Date().toISOString() + ': hello world')
publication.on('error', console.error)
}, 1000)
}()) |
I am going to test the new broker these days to see if there are any problems :P I will make PR later |
Looks good to me. Thanks. After submitting the PR how do you feel about maintaining a 'rascal-as-promised' module? |
Thanks, I might not be a good person to maintain that since most of the time I am not working with something related to AMQP :P I prefer to making rascal friendly to promisify tools so that users who want to use a promise style API can easily achieve that while rascal needn't care much about the promise API. |
Ok seems that the promise way may lose message..caused by |
Good spot. It's a weakness in the rascal API that you can't safely go async between calling Couple of things I might be able to do...
|
Not as easy as I'd hoped.
|
Agree with you. BTW Promise is at least a microtask so that changing API is unavoidable to support Promise. This functionality is more difficult than I thought :P |
I think the API will be better if the user can subscribe to events before starting to consume messages from rabbit, so I'm still giving it some thought. I have a lot of other stuff on at the moment though so it won't be soon though. |
@BorePlusPlus just came up with a nice idea. Instead of consuming messages immediately, if rascal could wait for an |
Any change we can see this on the master branch? |
Thanks for the nudge @mixecan. I'll bump it up my list. |
Currently available on the promise-support branch. I'll publish once I've done some more testing and had it reviewed. |
Release as 4.0.0 |
No description provided.
The text was updated successfully, but these errors were encountered: