Allow connecting with a string instead of an object #17
Conversation
Also removes amqp-uri since the object functionality is built into amqplib
|
|
||
| module.exports = Binder; | ||
|
|
||
| function fixConnectionInfo(connectionInfo) { |
There was a problem hiding this comment.
This looks like a copy/paste of some non-trivial code. Can you consolidate it?
|
All the integration tests fail for me. I don't see any documentation about being able to pass an object into the connect method of amqplib. Are you sure that works? |
|
We ran into the same issue as this guy last night – which led me to this |
|
I see why the integration tests fail for me. The _test-env file has a non-standard user and vhost. Was that intentional or left in by mistake? |
|
What do your tests say when they fail? Mine work, but I'm running them in docker with |
|
Oops - yeah that was a mistake I had because I was verifying the connection details worked. I fixed that. |
|
If you want to be compatible with amqp-uri, fixConnectionInfo should translate the ssl property to the protocol property. I think that's specified as false in all our apps that use this so far. See https://github.com/LeisureLink/legoland-adapter-api/blob/develop/lib/listeners/index.js If you don't care about compatibility or causing breaking changes for those projects (they should all upgrade to 1.0 anyway) then I'd say get rid of fixConnectionInfo and just accept the amqplib format. |
|
Yeah you're right, we should just pass that through. I'll remove it and make a note in the docs. |
|
Ok the docs are updated to work with a string instead of an object. Seems like it's more clear that way anyway. |
| @@ -16,10 +16,10 @@ describe('Broker', function() { | |||
| var serviceDomainName = 'my-domain'; | |||
| var appName = 'my-app'; | |||
| var connectionInfo = { | |||
There was a problem hiding this comment.
Not important for these tests, but this is an inaccurate example of connectionInfo. Should be hostname and username.
Allow connecting with a string instead of an object
Also removes amqp-uri since the object functionality is built into amqplib