feat(session): add support redis session storage#142
feat(session): add support redis session storage#142AlbumenJ merged 3 commits intoagentscope-ai:mainfrom
Conversation
|
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
AlbumenJ
left a comment
AlbumenJ
left a comment
There was a problem hiding this comment.
Please add license header & remove all the chinese comments
There was a problem hiding this comment.
Pull request overview
This PR adds Redis-based session storage support to AgentScope-Java, implementing two Redis client options: Redisson and Jedis. The implementation allows for distributed session management with persistent storage in Redis.
- Introduces a new extension module for Redis session storage
- Provides two implementation options: RedissonSession (using Redisson client) and JedisSession (using Jedis client)
- Includes comprehensive unit tests with mocked Redis clients
- Adds necessary dependencies (Redisson 3.37.0 and Jedis 5.1.0) to the BOM
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| agentscope-extensions/pom.xml | Adds the new session-redis module to the extensions parent POM |
| agentscope-extensions/agentscope-extensions-session-redis/pom.xml | Defines dependencies for the new Redis session extension module |
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/redisson/RedissonSession.java | Implements Redis-based session storage using the Redisson client with JSON serialization |
| agentscope-extensions/agentscope-extensions-session-redis/src/main/java/io/agentscope/core/session/redis/jedis/JedisSession.java | Implements Redis-based session storage using the Jedis client with JSON serialization |
| agentscope-extensions/agentscope-extensions-session-redis/src/test/java/io/agentscope/core/session/redis/RedissonSessionTest.java | Unit tests for RedissonSession with mocked Redis operations |
| agentscope-extensions/agentscope-extensions-session-redis/src/test/java/io/agentscope/core/session/redis/JedisSessionTest.java | Unit tests for JedisSession with mocked Redis operations |
| agentscope-dependencies-bom/pom.xml | Adds Redisson and Jedis dependency versions to the dependency management BOM |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| class RedissonSessionTest { | ||
|
|
||
| private RedissonClient redissonClient; | ||
| // 使用原始类型以避免 Mockito 泛型擦除带来的编译问题 |
There was a problem hiding this comment.
Comment contains Chinese text that should be translated to English. The comment explains "Use raw types to avoid Mockito generic erasure compilation issues." Consider translating to: // Use raw types to avoid Mockito generic erasure compilation issues
| // 使用原始类型以避免 Mockito 泛型擦除带来的编译问题 | |
| // Use raw types to avoid Mockito generic erasure compilation issues |
| Set<String> keys = jedis.keys(keyPrefix + "*"); | ||
| for (String key : keys) { | ||
| if (key.endsWith(META_SUFFIX)) { | ||
| continue; | ||
| } | ||
| String sessionId = extractSessionId(key); | ||
| if (sessionId != null) { | ||
| sessionIds.add(sessionId); | ||
| } | ||
| } |
There was a problem hiding this comment.
The Jedis keys() command is a blocking operation that scans all keys in Redis and can cause performance issues in production environments with large datasets. Consider using SCAN command instead via jedis.scan() with a cursor to iterate over keys in a non-blocking manner, especially for production use cases.
| Set<String> keys = jedis.keys(keyPrefix + "*"); | |
| for (String key : keys) { | |
| if (key.endsWith(META_SUFFIX)) { | |
| continue; | |
| } | |
| String sessionId = extractSessionId(key); | |
| if (sessionId != null) { | |
| sessionIds.add(sessionId); | |
| } | |
| } | |
| String cursor = "0"; | |
| do { | |
| redis.clients.jedis.ScanResult<String> scanResult = jedis.scan(cursor, keyPrefix + "*"); | |
| List<String> keys = scanResult.getResult(); | |
| for (String key : keys) { | |
| if (key.endsWith(META_SUFFIX)) { | |
| continue; | |
| } | |
| String sessionId = extractSessionId(key); | |
| if (sessionId != null) { | |
| sessionIds.add(sessionId); | |
| } | |
| } | |
| cursor = scanResult.getCursor(); | |
| } while (!"0".equals(cursor)); |
| try (Jedis jedis = getJedisResource()) { | ||
| int deletedSessions = 0; | ||
|
|
||
| Set<String> keySet = jedis.keys(keyPrefix + "*"); |
There was a problem hiding this comment.
The Jedis keys() command is a blocking operation that scans all keys in Redis and can cause performance issues in production environments with large datasets. Consider using SCAN command instead via jedis.scan() with a cursor to iterate over keys in a non-blocking manner, especially for production use cases.
| * JedisSession session = JedisSession.builder() | ||
| * .host("127.0.0.1") | ||
| * .port(6379) |
There was a problem hiding this comment.
The usage example shows builder methods .host("127.0.0.1") and .port(6379) that don't exist in the Builder class. The builder only has .jedisPool(JedisPool) and .keyPrefix(String) methods. Update the example to show the correct usage:
JedisPool jedisPool = new JedisPool("127.0.0.1", 6379);
JedisSession session = JedisSession.builder()
.jedisPool(jedisPool)
.keyPrefix("agentscope:session:")
.build();| * JedisSession session = JedisSession.builder() | |
| * .host("127.0.0.1") | |
| * .port(6379) | |
| * JedisPool jedisPool = new JedisPool("127.0.0.1", 6379); | |
| * JedisSession session = JedisSession.builder() | |
| * .jedisPool(jedisPool) |
| * RedissonSession session = RedissonSession.builder() | ||
| * .address("redis://127.0.0.1:6379") |
There was a problem hiding this comment.
The usage example shows a builder method .address("redis://127.0.0.1:6379") that doesn't exist in the Builder class. The builder only has .redissonClient(RedissonClient) and .keyPrefix(String) methods. Update the example to show the correct usage:
Config config = new Config();
config.useSingleServer().setAddress("redis://127.0.0.1:6379");
RedissonClient client = Redisson.create(config);
RedissonSession session = RedissonSession.builder()
.redissonClient(client)
.keyPrefix("agentscope:session:")
.build();| * RedissonSession session = RedissonSession.builder() | |
| * .address("redis://127.0.0.1:6379") | |
| * Config config = new Config(); | |
| * config.useSingleServer().setAddress("redis://127.0.0.1:6379"); | |
| * RedissonClient client = Redisson.create(config); | |
| * RedissonSession session = RedissonSession.builder() | |
| * .redissonClient(client) |
Thank you very much for your CR. I have made corresponding changes to the problems encountered during the CR process. Thank you very much again for your CR |
@AlbumenJ Signed |
4 participants
AgentScope-Java Version
[The version of AgentScope-Java you are working on, e.g. 1.0.0, check your pom.xml dependency version or run
mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]1.0.0
mac os
Description
[Please describe the background, purpose, changes made, and how to test this PR]
add support redis session storage
#128
Checklist
Please check the following items before code is ready to be reviewed.
mvn spotless:applymvn test)