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

Prioritize hash in connection parameter detection #153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tagliala
Copy link

@tagliala tagliala commented 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 #152

@tagliala tagliala force-pushed the bugfix/152-allow-as-orderedoptions branch from dcefdb0 to c88c8ce Compare November 21, 2024 10:03
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
@tagliala
Copy link
Author

rspec ./spec/client_spec.rb:122 # Redlock::Client lock when lock is available when redis connection error occurs fails to acquire a lock if majority of Redis instances are not available

Is failure related to this change?

Also, I can see an issue with coveralls

[Coveralls] Submitting to https://coveralls.io/api/v1
Coveralls encountered an exception:
OpenSSL::SSL::SSLError

@leandromoreira
Copy link
Owner

rspec ./spec/client_spec.rb:122 # Redlock::Client lock when lock is available when redis connection error occurs fails to acquire a lock if majority of Redis instances are not available

Is failure related to this change?

I believe so

Also, I can see an issue with coveralls

I don't think that's blocking PR error-like.

[Coveralls] Submitting to https://coveralls.io/api/v1
Coveralls encountered an exception:
OpenSSL::SSL::SSLError

@tagliala
Copy link
Author

I can also work on coveralls integration in a separate PR, I manage to maintain coveralls ruby reborn which is not required for GitHub actions

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.

Redlock and Rails: issues with how the connection param is being detected
2 participants