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

direct reply-to & bug in the fields serialization #78

Open
AVVS opened this issue Jun 18, 2017 · 4 comments
Open

direct reply-to & bug in the fields serialization #78

AVVS opened this issue Jun 18, 2017 · 4 comments

Comments

@AVVS
Copy link
Contributor

AVVS commented Jun 18, 2017

Hey,

I've been playing around with RPC modes & priority queues and noticed that there is a bug:

https://github.com/dropbox/amqp-coffee/blob/master/src/lib/Connection.coffee#L438

Specifically if value of the field is 0

  if args[field.name]

that check wont pass and there would be inconsistency between headers payload and actual payload (example, priority: 0). That being said we should simply check for hasOwnProperty to fix that issue

On the second note - https://www.rabbitmq.com/direct-reply-to.html - this is a nice feature to have, but require that publisher & consumer uses the same channel, which is currently not possible as they are always on the separate channels. I looked at how to do it myself, but didn't come to any conclusion - could be a decent idea to write an RPC channel with capabilities of both, but then I wouldnt want to duplicate a lot of code from Publisher/Consumer channels

What do you think is the best way to implement it?

@barshow
Copy link
Contributor

barshow commented Jun 19, 2017

Good catch on the args[field.name], I think the fix there is just if args[field.name]? I am happy to cut a diff or merge a PR in.

I am playing with direct reply now I'll update this later today:) I would hope this should be easy to use

@barshow
Copy link
Contributor

barshow commented Jun 19, 2017

published the pr, as far as DirectReplies give me a couple days and I think I can have something working, the issue is channel sharing as you pointed out. The direction I was going to go was make a rpc channel like you suggested the extends either publish or consume (not sure which is easier) and then reference the other as a object method. I want to make sure the rpc channel gets cleaned up at the right time, like if there are no outstanding requests or something like that. This is a really nice feature to have so if you dont take a crack at it I will for sure

@AVVS
Copy link
Contributor Author

AVVS commented Nov 30, 2017

@barshow any thoughts on that? I might have time to tackle it myself, but then again I'd actually want to go through most sources and migrate it to something more manageable than coffee script

es6+flow or ts comes to my mind, what do you think about it? At the same time we could do a more straightforward implementation for direct reply than copying a lot of logic

@barshow
Copy link
Contributor

barshow commented Dec 12, 2017

Sorry for the super long delay, I'm actually no longer at Dropbox however I do think that moving to something like es6 would be great.

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

No branches or pull requests

2 participants