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

make platform configurable #80

Closed
wants to merge 1 commit into from

Conversation

mafintosh
Copy link

Hi.

We use this module across multiple services and would like to use the platform string to track which services are connected to our rabbit cluster.

This PR allows you to configure platform by passing it as an option to connect

@michaelklishin
Copy link

While this is a fine feature to have (and other clients generally allow client properties configuration), I'd like to add that this is not really what the platform key is for. I'd say information or even product are more appropriate.

@mafintosh
Copy link
Author

Alright - I'll change it to product :)

@mafintosh
Copy link
Author

There you go. product and version are now configurable instead of platform. This solves my problem as well :)

@squaremo
Copy link
Collaborator

squaremo commented Jul 2, 2014

The client properties are arbitrary, but the spec (https://www.rabbitmq.com/resources/specs/amqp0-9-1.xml) suggests that the 'product', 'platform', and 'version' of the client should be included. I take e.g., "product", o be the name of the client library, i.e., "amqplib"; likewise "version". (Arguably "platform" should be the OS rather than the version of Node.JS, or a combination of those.) In other words I treat it like a user agent string.

I can see why it's useful to include some other information so you can see it in the management console or whatever. I'd rather it was fields other than those already included, though -- they will still be encoded. Maybe "application" or "service", for your purposes.

@squaremo
Copy link
Collaborator

squaremo commented Jul 2, 2014

So just to be clear, I think an application should be able to supply arbitrary client properties, but those already in CLIENT_PROPERTIES cannot be overridden.

I don't think socketOptions is the right place to supply them, but on the other hand, I wouldn't want another argument to connect either, and it would be a pain to encode them in the URL. (Actually it's a pain to encode other options like heartbeat in the URL, but it's done now)

@squaremo
Copy link
Collaborator

The code in master branch now lets you supply client properties when you connect -- see #98 for an example.
As above, I'd rather you didn't override existing properties, since that loses information.

@squaremo
Copy link
Collaborator

The feature referred to above is in release v0.3.0.

@squaremo squaremo closed this Oct 27, 2014
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.

3 participants