[improve][Redis]Redis scan command supports versions 5, 6, 7 #7666
Conversation
… different maven modules
… different maven modules,pom add license header
| import java.util.Collections; | ||
| import java.util.List; | ||
|
|
||
| public class Redis3IT extends RedisTestCaseTemplateIT { |
There was a problem hiding this comment.
I think we can just put all IT like Redis3IT, Redis4IT into same moudle.
There was a problem hiding this comment.
I think we can just put all IT like
Redis3IT,Redis4ITinto same moudle.
The initial version was like this, but CI would run out of time
There was a problem hiding this comment.
So I didn't think of a great way to run so many versions of tests and ensure that CI runs without timeout
|
I checked redis version EOL. Maybe we should only keep test case 5, 6, 7? https://redis.io/docs/latest/operate/rs/installing-upgrading/product-lifecycle/ |
After my testing, the syntax of versions 3, 4, and 5 is consistent, and the features of versions 6 and 7 are consistent. So I only need to keep two versions of 5 and 7 for testing.which may avoid CI timeout |
As the conclusion of yesterday, the command of scan are different between redis5 and 6,7, and redis 6，7 are the same, so I wrote the test code in one module, and only reserved the two tests 5 and 7. |
FuYouJ
changed the title
| public static final Option<RedisVersion> REDIS_VERSION = | ||
| Options.key("redis_version") | ||
| .enumType(RedisVersion.class) | ||
| .noDefaultValue() |
There was a problem hiding this comment.
Please set default version to version 7.
There was a problem hiding this comment.
In the next commit, this configuration is removed because we get the version through the client.
| @@ -40,14 +44,42 @@ protected RedisClient(RedisParameters redisParameters, Jedis jedis) { | |||
| this.redisParameters = redisParameters; | |||
| this.batchSize = redisParameters.getBatchSize(); | |||
| this.jedis = jedis; | |||
| this.redisVersion = redisParameters.getRedisVersion(); | |||
There was a problem hiding this comment.
Can we get the version by use redis client api instead of get from users?
There was a problem hiding this comment.
ok，good idea. We can get the redis information through jedis.info(), so we can remove the configuration option for redis_version.
private int extractRedisVersion(Jedis jedis) {
log.info("Try to get redis version information from the jedis.info() method");
// # Server
// redis_version:5.0.14
// redis_git_sha1:00000000
// redis_git_dirty:0
String info = jedis.info();
try {
for (String line : info.split("\n")) {
if (line.startsWith("redis_version:")) {
// 5.0.14
String versionInfo = line.split(":")[1].trim();
log.info("The version of Redis is :{}", versionInfo);
String[] parts = versionInfo.split("\\.");
return Integer.parseInt(parts[0]);
}
}
} catch (Exception e) {
throw new RedisConnectorException(
GET_REDIS_VERSION_INFO_FAILED,
GET_REDIS_VERSION_INFO_FAILED.getErrorMessage(),
e);
}
throw new RedisConnectorException(
GET_REDIS_VERSION_INFO_FAILED,
"Did not get the expected redis_version from the jedis.info() method");
}
| #spark config | ||
| spark.app.name = "SeaTunnel" | ||
| spark.executor.instances = 2 | ||
| spark.executor.cores = 1 | ||
| spark.executor.memory = "1g" | ||
| spark.master = local |
There was a problem hiding this comment.
| #spark config | |
| spark.app.name = "SeaTunnel" | |
| spark.executor.instances = 2 | |
| spark.executor.cores = 1 | |
| spark.executor.memory = "1g" | |
| spark.master = local |
| Assertions.assertEquals(0, execResult.getExitCode()); | ||
| jedis.select(3); | ||
| Assertions.assertEquals(2, jedis.llen("key_multi_list")); | ||
| jedis.del("key_multi_list"); | ||
| jedis.select(0); | ||
| } | ||
|
|
||
| ///// need different redis version test override |
Purpose of this pull request
related bug issue #7664
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide
release-note.