Add option to use a different Connector. #591
Conversation
|
What would be the use case of this? |
|
I'm trying to run a custom connector without having to monkey patch. Basically have the ability to subclass In my case, I'm not using sentinals for HA. We run a master slave config and would like to have a custom connector that handles the failover. |
|
@badboy I would like to clarify if the request is reasonable :) |
|
@badboy any news on this? need to know if its acceptable or not, I hate carrying freedom patches around for Discourse :) |
|
@badboy any news on this issue? |
|
ping @djanowski Any thoughts about this? |
|
@badboy @djanowski Don't mean to ping multiple times but we would love to get some feedback on this. |
|
@byroot Was wondering if you might have any thoughts on this. |
lib/redis/client.rb
Outdated
| @connector = | ||
| if options.include?(:sentinels) | ||
| Connector::Sentinel.new(@options) | ||
| elsif options.include?(:connector) && options[:connector].is_a?(Class) |
There was a problem hiding this comment.
I wouldn't check for is_a?(Class), respond_to?(:new) would be better as it would allow for facades etc.
tgxworld
force-pushed
the
allow_custom_connectors
branch
from
8f06607 to
088b8c6
Compare
|
@byroot Thank you for reviewing. I've updated the PR as per your comments and added tests as well. |
|
@byroot Thank you for reviewing. Just curious, is Shopify/Rails taking up ownership of this gem now? |
|
A few people from Shopify have been added as collaborators on the repo indeed. We should also get Rubygems access some day so we can release new versions as well. |
|
Awesome 👍 Thanks for reviewing once again! |
No description provided.