Conversation
morrowgi
requested review from
davidartplus,
earlzero,
fantomfield,
jijisv,
johnmcclean and
tonyfinn
|
Implementing #355 |
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.scheduling.annotation.Scheduled; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Are we keeping these kind of comments in micro-server?
There was a problem hiding this comment.
woops will remove...
| @Slf4j | ||
| public class CouchbaseDistributedMapClient<V> implements DistributedMap<V> { | ||
|
|
||
| private final Logger logger = LoggerFactory.getLogger(getClass()); |
There was a problem hiding this comment.
Not necessary when you have the lombok @slf4j annotation
| private long opTimeout; | ||
|
|
||
| @Value("${distributed.cache.default.expiration:691200}") | ||
| private int expiresAfterSeconds = 691200; |
There was a problem hiding this comment.
Don't need to assign this value will be overwritten by Spring annotation same for line 52,55
There was a problem hiding this comment.
Ok removed the assignment
|
|
||
| } catch (final Exception e) { | ||
|
|
||
| log.warn("memcache put: {}", e.getMessage()); |
There was a problem hiding this comment.
I think you mean couchbase instead of memcache ?
| private ElasticacheConnectionTester elasticacheConnectionTester; | ||
|
|
||
|
|
||
| @Scheduled(fixedDelay = 60000) |
There was a problem hiding this comment.
Maybe expose this as a property ? but have it default to 60000
There was a problem hiding this comment.
Changed as @scheduled expects a constant I needed to assign it directly in code
| try { | ||
| result = testConnection(); | ||
| } catch (RuntimeException e) { | ||
| log.debug("Could not connect to Cache" + e.getMessage()); |
There was a problem hiding this comment.
is this log at the right level ? also line 41
There was a problem hiding this comment.
I was keeping the convention as it was but I think you are right I will log as error if it cannot connect...
|
|
||
| private final String key; | ||
|
|
||
| private volatile Xor<Void, T> data = Xor.secondary(null); // Void represents |
There was a problem hiding this comment.
Why Xor<Void, T> over just Optional<T>? Or if you're worried about the loaded/not loaded yet case seperately, Future<Optional<T>> ?
There was a problem hiding this comment.
Ok I think I can convert to Optional
| return currentVersionedKey.withVersion(currentVersionedKey.getVersion() + 1); | ||
| } | ||
|
|
||
| private VersionedKey loadKeyFromCouchbase() { |
There was a problem hiding this comment.
It's not coming from Couchbase here.
|
|
||
| import java.util.Optional; | ||
|
|
||
| public interface DistributedCache<V> { |
There was a problem hiding this comment.
Can you inherit DistributedMap here?
| public class CouchbaseResource { | ||
|
|
||
| private final DistributedMap client; | ||
| private final DistributedCache client; |
There was a problem hiding this comment.
I'd change the reference name as well 😃
6 participants
No description provided.