-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Introduce new client classes #4355
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new, cleaner client API for Jedis by creating three new client classes (RedisClient, RedisClusterClient, and RedisSentinelClient) that replace the legacy client classes (JedisPooled, JedisCluster, and JedisSentineled). The new classes use a builder pattern for advanced configuration while maintaining simple constructors for basic use cases.
- Introduced new client classes with builder pattern support for flexible configuration
- Deprecated legacy client classes and UnifiedJedis constructors with migration guidance
- Updated internal references to use the new
RedisClusterClient.INIT_NO_ERROR_PROPERTYconstant - Added comprehensive integration tests demonstrating migration paths from legacy constructors
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| RedisClient.java | New standalone Redis client with simple constructors and builder pattern support |
| RedisClusterClient.java | New Redis Cluster client replacing JedisCluster with builder pattern |
| RedisSentinelClient.java | New Redis Sentinel client replacing JedisSentineled |
| JedisPooled.java | Added deprecation notice directing users to RedisClient |
| JedisCluster.java | Added deprecation notice directing users to RedisClusterClient |
| JedisSentineled.java | Added deprecation notice directing users to RedisSentinelClient |
| UnifiedJedis.java | Deprecated multiple constructors and removed unused import |
| ClusterClientBuilder.java | Removed generic type constraint to support both legacy and new cluster clients |
| ClusterConnectionProvider.java | Updated INIT_NO_ERROR_PROPERTY reference to use RedisClusterClient |
| JedisClusterInfoCache.java | Updated INIT_NO_ERROR_PROPERTY reference to use RedisClusterClient |
| RedisClusterClientMigrationIntegrationTest.java | Comprehensive integration tests validating migration from JedisCluster to RedisClusterClient |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bf56e75 to
be9dc0e
Compare
Test Results 283 files +1 283 suites +1 11m 40s ⏱️ -8s Results for commit 85c881f. ± Comparison against base commit bce2ccc. This pull request removes 1450 and adds 1457 tests. Note that renamed tests count towards both.This pull request removes 387 skipped tests and adds 66 skipped tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…tor and other OSS projects
Provide clear guidance which client class should be used instead.
d401cac to
066e34c
Compare
Co-authored-by: Ivo Gaydazhiev <ivo.gaydazhiev@redis.com>
ggivo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Refined version of #4201
RedisClient,RedisClusterClient,RedisSentinelClient) that provide a clean, developer-friendly API while addressing critical developer experience issues identified in the current Jedis architecture.Deprecated Class Mappings
JedisPoolRedisClientJedisSentinelPoolRedisSentinelClientJedisPooledRedisClientJedisSentineledRedisSentinelClientJedisClusterRedisClusterClientJedisPoolConfigConnectionPoolConfigJedisFactoryUnifiedJedis(constructors)RedisClient/RedisSentinelClient/RedisClusterClient