shedlock-provider-redis-quarkus2 impl using vert.x redis client #1553
Conversation
.../src/main/java/net/javacrumbs/shedlock/provider/redis/quarkus2/QuarkusRedisLockProvider.java
Outdated
Show resolved
Hide resolved
.../src/main/java/net/javacrumbs/shedlock/provider/redis/quarkus2/QuarkusRedisLockProvider.java
Outdated
Show resolved
Hide resolved
.../src/main/java/net/javacrumbs/shedlock/provider/redis/quarkus2/QuarkusRedisLockProvider.java
Outdated
Show resolved
Hide resolved
|
|
||
| long expireTime = getSecsUntil(lockConfiguration.getLockAtMostUntil()); | ||
|
|
||
| String key = buildKey(lockConfiguration.getName(), this.environment); |
There was a problem hiding this comment.
Do not need to pass instance variable environment as an argument?
There was a problem hiding this comment.
I didn't understand your point
|
|
||
| public QuarkusRedisLockProvider(RedisDataSource dataSource, String appNameOrPrefix) { | ||
| this.redisDataSource = dataSource; | ||
| this.environment = appNameOrPrefix + ":"+ String.join(":", ConfigUtils.getProfiles()); |
There was a problem hiding this comment.
Can you please explain why the enviroment is used for the key. I do not think we have anything like this in any other lock provider.
There was a problem hiding this comment.
Ditto the appNameOrPrefix. If you have a use-case for it, let's just keep the appNameOrPrefix.
There was a problem hiding this comment.
Part of this code was copied from the spring implementation, it used the profile/environment name to compose the key. I ended up leaving it as it was.
|
|
||
|
|
||
| @ApplicationScoped | ||
| public class SchedulerLockFactory { |
There was a problem hiding this comment.
Do we need this? In Spring we let the users to specify the Lock Provider in the aplication code. Having it in the library feels surprising.
There was a problem hiding this comment.
I think if I import a specific provider it's because I want to use it... I don't see the need to configure it manually
.../net/javacrumbs/shedlock/provider/redis/quarkus/QuarkusRedisLockProviderIntegrationTest.java
Outdated
Show resolved
Hide resolved
|
I have not realized that it's for an old version of Quarkus, sorry. In this case I will not be willing to merge it as it will be obsolete the second we release it. Thanks again for your contribution, but in this case it will be better if you keep the code only in your codebase. |
|
This is "oficial version" from redhat |
|
Would something like this be OK for you? |
Implementation for quakus 2x using native quarkus redis client