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

Redlock and Rails: issues with how the connection param is being detected #152

Open
tagliala opened this issue Nov 19, 2024 · 0 comments · May be fixed by #153
Open

Redlock and Rails: issues with how the connection param is being detected #152

tagliala opened this issue Nov 19, 2024 · 0 comments · May be fixed by #153

Comments

@tagliala
Copy link

tagliala commented Nov 19, 2024

Hello,

I've searched for existing issues, apologies if this is a duplicate

We had some troubles using Redlock in a Rails application because of how the server parameter is being detected:

if connection.respond_to?(:call)
@redis = connection
else
if connection.respond_to?(:client)
@redis = connection
elsif connection.respond_to?(:key?)
@redis = initialize_client(connection)
else
@redis = connection
end
end

> redis_config = Rails.application.config_for(:redis)
=> {:name=>"master", :url=>"redis://localhost:6379/4", :password=>"pass", :sentinels=>[{:host=>"localhost", :port=>26379, :password=>nil}]}

> Redlock::Client.new([redis_config]).lock!('test', 1000) { 'Ok' }
Redlock::LockError: failed to acquire lock on 'test'

This does not work in Rails by default because redis_config is an ActiveSupport::OrderedOptions object which responds to call, tricking initialize into thinking that it is not a configuration hash.

In fact, by adding to_h, everything is fine

Redlock::Client.new([redis_config.to_h]).lock!('test', 1000) { 'Ok' }
=> "Ok"

I wonder if there could be a different approach, something like:

@redis =
  if connection.is_a?(Hash)
    initialize_client(connection)
  else
    connection
  end

It appears to me that the minimum required Ruby version is 2.5.0 because of the runtime requirement on https://rubygems.org/gems/redis-client/versions/0.14.1

Ruby 2.5.0 defines key? on hash: https://docs.ruby-lang.org/en/2.5.0/Hash.html#method-i-key-3F

I understand that is_a?(Hash) is not the same thing as key?, but in initialize_client, options is being used like a hash with:

  • #delete
  • #[]
  • ** operator

So I guess that this may be a legit change, unless some other options inherit from Hash, which I hope they aren't

I can submit a PR if you are interested

@tagliala tagliala changed the title Redlock and rails: change how the Redlock and Rails: issues with how the connection param is being detected Nov 19, 2024
tagliala added a commit to tagliala/redlock-rb that referenced this issue Nov 21, 2024
Allow to detect hashes responding to `#call`, like 

This will enable out of the box compatibility with
`Rails.application.config_for` method and should not break existing
use cases

Close leandromoreira#152
@tagliala tagliala linked a pull request Nov 21, 2024 that will close this issue
tagliala added a commit to tagliala/redlock-rb that referenced this issue Nov 21, 2024
Allow to detect hashes responding to `#call`, like
`ActiveSupport::OrderedOptions`

This will enable out of the box compatibility with
`Rails.application.config_for` method and should not break existing
use cases

Close leandromoreira#152
tagliala added a commit to tagliala/redlock-rb that referenced this issue Nov 21, 2024
Allow to detect hashes responding to `#call`, like
`ActiveSupport::OrderedOptions`

This will enable out of the box compatibility with
`Rails.application.config_for` method and should not break existing
use cases, because `ConnectionPool` and `RedisClient` do not inherit
from `Hash`

Close leandromoreira#152
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 a pull request may close this issue.

1 participant