Skip to content

Conversation

@stanhu
Copy link
Contributor

@stanhu stanhu commented Jan 8, 2026

PR #277 fixed the issue where ssl was not set on the Redis client automatically if a rediss:// URL were used:

require 'redis'

config = {
    url: 'rediss://mymaster',
    sentinels: [
      { host: 'localhost', port: 26379, ssl: true } ,
      { host: 'localhost', port: 26381, ssl: true },
      { host: 'localhost', port: 26380, ssl: true }
    ]
  }
  client = Redis.new(config)

However, it broke the case if you explicitly set ssl and expected for it to propagate:

require 'redis'

config = {
    name: 'mymaster',
    ssl: true,
    sentinels: [
      { host: 'localhost', port: 26379 },
      { host: 'localhost', port: 26381 },
      { host: 'localhost', port: 26380 }
    ]
  }
  client = Redis.new(config)

Closes #279

@stanhu stanhu force-pushed the sh-fix-issue-279 branch 5 times, most recently from 0ae2399 to 2312db7 Compare January 8, 2026 08:14
@stanhu stanhu marked this pull request as draft January 8, 2026 08:17
@stanhu
Copy link
Contributor Author

stanhu commented Jan 8, 2026

This is still not right...

PR redis-rb#277 fixed the issue where `ssl` was not set on the Redis client
automatically if a `rediss://` URl were used:

```ruby
require 'redis'

config = {
    url: 'rediss://mymaster',
    sentinels: [
      { host: 'localhost', port: 26379, ssl: true } ,
      { host: 'localhost', port: 26381, ssl: true },
      { host: 'localhost', port: 26380, ssl: true }
    ]
  }
  client = Redis.new(config)
````

However, it broke the case if you explicitly set `ssl` and expected it
to propagate:

```ruby
require 'redis'

config = {
    name: 'mymaster',
    ssl: true,
    sentinels: [
      { host: 'localhost', port: 26379 },
      { host: 'localhost', port: 26381 },
      { host: 'localhost', port: 26380 }
    ]
  }
  client = Redis.new(config)
````

Closes redis-rb#279
@stanhu stanhu changed the title Only set SSL parameter if it's not set already Only set SSL parameter if it's not explicitly set Jan 8, 2026
@stanhu stanhu marked this pull request as ready for review January 8, 2026 08:40
@stanhu stanhu requested a review from IevaGraz January 8, 2026 08:40
@byroot byroot merged commit 2b044db into redis-rb:master Jan 9, 2026
15 checks passed
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.

Allow to pass ssl option for sentinel with params

3 participants