redis-py icon indicating copy to clipboard operation
redis-py copied to clipboard

Fix sentinel connections

Open garionphx opened this issue 1 year ago • 4 comments

When passing args to Redis(), using the connection_kwargs args allows us to use authentication and other connection features.

Pull Request check-list

Please make sure to review and check all of these items:

  • [x] Do tests and lints pass with this change?
  • [ ] Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
  • [ ] Is the new or changed code fully tested?
  • [ ] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
  • [x] Is there an example added to the examples folder (if applicable)?
  • [x] Was the change added to CHANGES file?

NOTE: these things are not required to open a PR and can be done afterwards / while the PR is open.

Description of change

Modified the arguments passed to Redis() when forming Sentinel connections

garionphx avatar Sep 10 '24 04:09 garionphx

great, thank you!

garry-t avatar Sep 10 '24 06:09 garry-t

@garionphx Thanks for your contribution! Make sure that tests passes

vladvildanov avatar Sep 12 '24 07:09 vladvildanov

@garionphx Since this changes makes self.sentinel_kwargs argument completely redundant, it should be removed from constructor properties and it will be a breaking change.

vladvildanov avatar Sep 12 '24 08:09 vladvildanov

This doesn't seem correct. The sentinel_kwargs are the connection_kwargs passed in to make the sentinel connections and the connection_kwargs are those used for making Redis connections. The arguments can be different for each and should be allowed to be.

rad-pat avatar Oct 15 '24 11:10 rad-pat

Hi @garionphx,

Thanks for the contribution! We should keep sentinel_kwargs and connection_kwargs as separate arguments. These serve distinct purposes: we may need different connection settings for Sentinel nodes and for Redis servers (both master and slaves). For example, SSL configuration, authentication credentials, or even different users might vary between the two connection types.

Keeping them as separate dictionaries is currently the only flexible and clean way to support these differences. That said, you can still supply usernames and credentials specifically for Sentinel connections via sentinel_kwargs.

For these reasons, I am going to close this PR.

petyaslavova avatar Jun 11 '25 08:06 petyaslavova