ruby-amqp / amq-protocol

AMQP 0.9.1 protocol serialization and deserialization implementation for Ruby (2.0+)
http://groups.google.com/group/ruby-amqp
MIT License
48 stars 31 forks source link

URI Query Parameters #66

Closed Tensho closed 6 years ago

Tensho commented 6 years ago

I found nor bunny neither amqp-protocol support URI query parameters in the connection string. Particularly AMQ::URI.parse method just grabs connection vital options. Is it intentionally?

It's easier to configure one env var instead of several (host, port, vhost, etc). Particularly, I would like to provide client heartbeat value within connection string instead of parameters map.

michaelklishin commented 6 years ago

There's been little demand for entirely URI-driven configuration so far. I'd be happy to accept a PR with improvements as long as someone can first demonstrate what it would look like.

Tensho commented 6 years ago

@michaelklishin, I'd happy to create PR, but first I would like to specify URI options here and confirm it with you. I will base on RabbitMQ official documentation "URI query parameters" page and Python RabbitMQ client Pika "Connection Parameters" page. If you know any other valuable related resources, please don't hesitate to refer me.

michaelklishin commented 6 years ago

Some parameters are client-specific, others we don't care about. See Bunny::Session for the list of what's actually being used. Bunny::Transport is another place I'd look at.

michaelklishin commented 6 years ago

Realistically you can only support values that are numerical, strings or symbols (that can get pretty confusing, fast), so I'd ignore stuff such as client properties entirely. The guide on rabbitmq.com lists options that are supported by the Erlang client and therefore Federation and Shovel plugins. I don't think it's essential for Bunny to use the same names but it'd be nice.

Tensho commented 6 years ago

Sure, I will consult Bunny::Session and Bunny::Transport modules and try to understand them :) I agree about compliance with the set of parameters RMQ Erlang client uses itself, but just in sake of my enlightenment could you give an example of client-specific (bunny) parameter?

michaelklishin commented 6 years ago

Channel operation timeout is not on the URI reference, fail_if_no_peer_cert is not an option you can configure in Bunny (although enabling peer verification effectively does that).

Tensho commented 6 years ago

Thank you for the quick feedback, @michaelklishin 🙇

Tensho commented 6 years ago
RabbitMQ Documentation Bunny::Session Description
cacertfile, certfile, keyfile tls_ca_certificates, tls_cert, tls_key Paths to files to use in order to present a client-side SSL certificate to the server. Only of use for the amqps scheme
verify, fail_if_no_peer_cert verify_peer Use to configure verification of the server's SSL certificate. See the SSL guide for details of SSL in RabbitMQ in general and specifically the Erlang client section. Only of use for the amqps scheme.
auth_mechanism auth_mechanism SASL authentication mechanisms to consider when negotiating a mechanism with the server. This parameter can be specified multiple times, e.g. ?auth_mechanism=plain&auth_mechanism=amqplain, to specify multiple mechanisms.
heartbeat heartbeat Heartbeat timeout value in seconds (an integer) to negotiate with the server.
connection_timeout connection_timeout Time in milliseconds (an integer) to wait while establishing a TCP connection to the server before giving up.
channel_max channel_max Maximum number of channels to permit on this connection.

So here, in amq-protocol gem, I'd like to introduce the parameters described above to conform RabbitMQ official documentation. I guess all other bunny specific parameters (read_timeout, write_timeout, recovery_attempts, etc.) should be parsed at bunny uri parsing level. Does such segregation make sense?

michaelklishin commented 6 years ago

@Tensho provided that we can just pass a hash around and merge the two, then use it as the existing hash of options, it does. amqp gem will then have to potentially be updated as well (even though it does not get much use and attention these days).

Tensho commented 6 years ago

Yes, I'd like to parse at different levels depending on parameters adoption and then merge common parameters hash and client specific parameters hash. I don't mind to update all 3 gems – amq-protocol, bunny, amqp.

michaelklishin commented 6 years ago

Addressed in #67. Thank you.

Tensho commented 6 years ago

Thank you, @michaelklishin 🙇