SentinelConnectionPool should retrieve updated master address from Sentinel on connection retries

[SamuelGD]

Description of change

#3874

Pull Request check-list

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

• 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)?
• Is there an example added to the examples folder (if applicable)?

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

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[petyaslavova]

Hi @SamuelGD, than you for your contribution! We will review it soon.

[Copilot]

Pull request overview

This PR refactors how SentinelManagedConnection handles connection retries to ensure that the master address is retrieved from Sentinel on each retry attempt. Previously, the connection retry logic was handled by a custom connect() method that didn't fetch fresh master addresses during retries.

Key changes:

• Refactored SentinelManagedConnection._connect() to fetch master/slave addresses directly and rely on parent class retry logic
• Removed custom connect(), connect_to(), and _connect_retry() methods in favor of overriding _connect()
• Added ConnectionError exception handling in async connection flow to preserve custom connection errors

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
redis/sentinel.py	Refactored SentinelManagedConnection._connect() to fetch master address on each call, removing custom retry wrapper
redis/asyncio/sentinel.py	Async version of the _connect() refactoring with equivalent logic
redis/asyncio/connection.py	Added explicit ConnectionError catch-and-rethrow to preserve custom connection errors
tests/test_sentinel_managed_connection.py	Updated tests to mock parent class _connect() and added new test for connect_check_health() retry behavior
tests/test_asyncio/test_sentinel_managed_connection.py	Async version of the test updates with equivalent assertions

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The logic here is incorrect. When self._sock exists, the connection is already established and the method should simply return without calling super()._connect(). The parent Connection._connect() returns a socket object, which would be inconsistent with the rest of this method that doesn't return anything. This should be:

if self._sock:
    return  # already connected

Suggested change

	return super()._connect() # already connected
	return # already connected

[SamuelGD]

If the connection is already established, _connect is never called https://github.com/redis/redis-py/blob/v7.1.0/redis/connection.py#L852

[Copilot]

The logic here is incorrect. When self.is_connected is True, the connection is already established and the method should simply return without calling super()._connect(). The check should just return early:

if self.is_connected:
    return None  # already connected

Calling await super()._connect() when already connected could cause issues as it attempts to set self._reader and self._writer again.

Suggested change

	await super()._connect()
	return None
	return None # already connected