Skip to content
This repository has been archived by the owner on Apr 2, 2019. It is now read-only.

Allow specification of multiple hosts #26

Merged
merged 2 commits into from
Jul 6, 2015
Merged

Allow specification of multiple hosts #26

merged 2 commits into from
Jul 6, 2015

Conversation

joshuafleck
Copy link

The purpose of this change is to allow us to forgo the load balancer and allow bunny to manage the hosts directly. This allows us to pass a list of hosts to bunny, of which it will choose one to connect and will failover upon lost connection.

@sparrovv @calo81 @daniel-barlow @PaulMcAdam @jaynefox @cpoo22 Could you please sign off?

@@ -2,13 +2,14 @@ module RabbitFeed
class Configuration
include ActiveModel::Validations

attr_reader :host, :port, :user, :password, :application, :environment, :exchange, :pool_size, :pool_timeout, :heartbeat, :connect_timeout, :network_recovery_interval, :auto_delete_queue, :auto_delete_exchange
attr_reader :host, :hosts, :port, :user, :password, :application, :environment, :exchange, :pool_size, :pool_timeout, :heartbeat, :connect_timeout, :network_recovery_interval, :auto_delete_queue, :auto_delete_exchange
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is singular host still relevant?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for connecting when there is not a cluster

@sparrovv
Copy link
Contributor

sparrovv commented Jul 6, 2015

I think it makes sense to update README with that new setting.

@@ -42,6 +43,7 @@ def connection_options
options[:heartbeat] = heartbeat if heartbeat
options[:connect_timeout] = connect_timeout if connect_timeout
options[:host] = host if host
options[:hosts] = hosts if hosts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, ask cause I don't know how this works... is this property being picked up automatically by Bunny?.. I mean where is this value actually used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calo81

is this property being picked up automatically by Bunny?

Yes

I mean where is this value actually used?

connection_options are what is passed to Bunny, see: https://github.com/simplybusiness/rabbit_feed/blob/master/lib/rabbit_feed/connection_concern.rb#L53

Bunny is what is actually uses the configs. RabbitFeed allows you to specify configs in yaml format and passes them to Bunny for you.

@joshuafleck
Copy link
Author

@sparrovv

I think it makes sense to update README with that new setting.

Added in 516e6fa, thanks!

@calo81
Copy link
Contributor

calo81 commented Jul 6, 2015

ok.. thanks for letting me know. It all looks good to me... What about the codeclimate in red?. anything to be aware of or is it being like that always?

@joshuafleck
Copy link
Author

What about the codeclimate in red?. anything to be aware of or is it being like that always?

Codeclimate appears to be having connectivity issues with github, as @gzzsound and I encountered this on another project today.

joshuafleck pushed a commit that referenced this pull request Jul 6, 2015
Allow specification of multiple hosts
@joshuafleck joshuafleck merged commit 404f22f into master Jul 6, 2015
@joshuafleck joshuafleck deleted the hosts branch July 6, 2015 12:45
@sparrovv
Copy link
Contributor

sparrovv commented Jul 6, 2015

Signed-off-by: @sparrovv

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants