Add Redis Sentinel support to prefect-redis#22377
Conversation
fatih-acar
requested review from
chrisguidry,
cicdw,
desertaxle and
zzstoatzz
as code owners
![chatgpt-codex-connector[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 989a3144b0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| def is_sentinel_url(url: str) -> bool: | ||
| """Return True if the URL uses a Redis Sentinel scheme.""" | ||
| return urlparse(url).scheme in SCHEME_SENTINEL |
There was a problem hiding this comment.
When the Sentinel member list includes an IPv6 host alongside another member (for example redis+sentinel://s1:26379,[::1]:26379/mymaster), urllib.parse.urlparse raises ValueError: Invalid IPv6 URL before the new tolerant parse_redis_url path can run. That makes get_async_redis_client reject a URL shape that _split_connection_url and the parser tests explicitly support; detect the scheme without stdlib netloc validation or reuse the shared splitter here.
Useful? React with 👍 / 👎.
| raise RedisUrlError( | ||
| f"Invalid database index {segment!r} in connection URL: {redact_redis_url(url)}" | ||
| ) from exc | ||
| if not 0 <= db <= 15: |
There was a problem hiding this comment.
When a Redis or Sentinel deployment is configured with more than the default 16 logical databases, URLs such as redis+sentinel://s1:26379/mymaster/16 are valid for the server/redis-py but this parser rejects them before connecting. Because this parser now gates the new connection_url paths, those deployments cannot select their configured database; only reject non-integer or negative values and let Redis report an unavailable DB.
Useful? React with 👍 / 👎.
Merging this PR will not alter performance
Comparing 
Footnotes
|
Build on the Redis Cluster URL groundwork merged in PrefectHQ#22211 to add native Redis Sentinel support. Cluster URLs stay gated (detection-only) exactly as in PrefectHQ#22211; this change enables the redis+sentinel:// and rediss+sentinel:// schemes across every connection point: the server messaging client, the RedisLockManager, the RedisDatabase block, and the Docket services URL. - New prefect_redis.connection module: a shared URL parser (parse_redis_url) and client builder (build_redis_client) that resolve the current master through the listed Sentinel daemons and follow failover automatically. Data-node connections get pinned TCP keepalive so a silently-dead master is noticed promptly while Sentinel completes failover. - client.py dispatches redis+sentinel:// URLs to the Sentinel builder while leaving PrefectHQ#22211's cluster NotImplementedError gate intact. - RedisLockManager and RedisDatabase accept a connection_url that is authoritative over the scalar host/port fields; RedisDatabase round-trips Sentinel URLs through from_connection_string and block_info. - Document the Sentinel schemes on PREFECT_SERVER_DOCKET_URL and bump the pydocket floor to >=0.22.0 for its Sentinel URL support. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
fatih-acar
force-pushed
the
feat/redis-sentinel-on-22211
branch
from
989a314 to
4b28891
Compare
|
hi team, any chance this gets accepted? it's an easier implementation than redis cluster, so hopefully it's not too much to review |
1 participant
Supersedes #22248, which targeted the
prefect-redis-cluster-client-foundationbranch from #22211; that branch has since merged tomain, auto-closing the old (draft) PR. This re-opens the work againstmain.This PR adds native Redis Sentinel support to
prefect-redis, building on the Redis Cluster URL groundwork merged in #22211. Cluster URLs stay detection-only (gated behindNotImplementedError) exactly as #22211 left them; this PR enables theredis+sentinel://andrediss+sentinel://schemes across every connection point: the server messaging client,RedisLockManager, theRedisDatabaseblock, and the Docket services URL.Details
prefect_redis.connectionmodule — a shared URL parser (parse_redis_url) and client builder (build_redis_client) that resolve the current master through the listed Sentinel daemons and follow failover automatically. Data-node (master) connections get pinned TCP keepalive so a silently-dead master is noticed promptly while Sentinel completes failover.client.pydispatchesredis+sentinel://URLs to the Sentinel builder while leaving Add Redis Cluster URL and key groundwork #22211's clusterNotImplementedErrorgate intact.RedisLockManagerandRedisDatabaseaccept aconnection_urlthat is authoritative over the scalar host/port fields;RedisDatabaseround-trips Sentinel URLs throughfrom_connection_stringandblock_info.PREFECT_SERVER_DOCKET_URLdocuments the Sentinel schemes; thepydocketfloor is bumped to>=0.22.0for its Sentinel URL support.URL grammar follows the
redis-sentinel-urlconvention:The Sentinel schemes accept a comma-separated member list and a master group name;
sentinel_username,sentinel_password,sentinel_ssl,sentinel_tls_insecure, andsentinel_tls_ca_filequery params configure the Sentinel daemon connections separately from the data nodes.Testing
prefect-redissuite passes (214 tests, incl. 46 new connection/integration tests).get_async_redis_client, andRedisLockManagerall automatically follow the Sentinel-promoted replica; data written before the crash survives via replication; the restarted old master rejoins as a replica.RedisDatabaseblock against a real Prefect server: block saved/loaded through the REST API, flow runs recordedCOMPLETED, andread_path/write_pathsurvive a master failover (Prefect task retries ride the promotion window).🤖 Generated with Claude Code