Repository: salesforce/Timbermill (language: python)

Issue:
Update test.yml



Discussion / hints:

Fix patch:
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/LocalCacheConfig.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/LocalCacheConfig.java
deleted file mode 100644
index 2f2c02b3..00000000
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/LocalCacheConfig.java
+++ /dev/null
@@ -1,19 +0,0 @@
-package com.datorama.oss.timbermill;
-
-public class LocalCacheConfig {
-    private final long maximumTasksCacheWeight;
-    private final long maximumOrphansCacheWeight;
-
-    public LocalCacheConfig(long maximumTasksCacheWeight, long maximumOrphansCacheWeight) {
-        this.maximumTasksCacheWeight = maximumTasksCacheWeight;
-        this.maximumOrphansCacheWeight = maximumOrphansCacheWeight;
-    }
-
-    public long getMaximumTasksCacheWeight() {
-        return maximumTasksCacheWeight;
-    }
-
-    public long getMaximumOrphansCacheWeight() {
-        return maximumOrphansCacheWeight;
-    }
-}
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/RedisCacheConfig.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/RedisCacheConfig.java
deleted file mode 100644
index 11913f0d..00000000
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/RedisCacheConfig.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package com.datorama.oss.timbermill;
-
-
-import com.datorama.oss.timbermill.common.redis.RedisService;
-
-public class RedisCacheConfig {
-    private final RedisService redisService;
-    private final int redisTtlInSeconds;
-
-    public RedisCacheConfig(RedisService redisService, int redisTtlInSeconds) {
-        this.redisService = redisService;
-        this.redisTtlInSeconds = redisTtlInSeconds;
-    }
-
-    public RedisService getRedisService() {
-        return redisService;
-    }
-
-    public int getRedisTtlInSeconds() {
-        return redisTtlInSeconds;
-    }
-}
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/TaskIndexer.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/TaskIndexer.java
index 3a2d0ac8..43e4affe 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/TaskIndexer.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/TaskIndexer.java
@@ -4,7 +4,6 @@
 import com.datorama.oss.timbermill.common.ElasticsearchUtil;
 import com.datorama.oss.timbermill.common.KamonConstants;
 import com.datorama.oss.timbermill.common.cache.AbstractCacheHandler;
-import com.datorama.oss.timbermill.common.cache.CacheHandlerUtil;
 import com.datorama.oss.timbermill.plugins.PluginsConfig;
 import com.datorama.oss.timbermill.plugins.TaskLogPlugin;
 import com.datorama.oss.timbermill.unit.*;
@@ -35,14 +34,12 @@ public class TaskIndexer {
     private long daysRotation;
     private String timbermillVersion;
 
-    public TaskIndexer(String pluginsJson, Integer daysRotation, ElasticsearchClient es, String timbermillVersion,
-                       LocalCacheConfig localCacheConfig, String cacheStrategy, RedisCacheConfig redisCacheConfig) {
-
+    public TaskIndexer(String pluginsJson, Integer daysRotation, ElasticsearchClient es, String timbermillVersion, AbstractCacheHandler cacheHandler) {
         this.daysRotation = calculateDaysRotation(daysRotation);
         this.logPlugins = PluginsConfig.initPluginsFromJson(pluginsJson);
         this.es = es;
         this.timbermillVersion = timbermillVersion;
-        cacheHandler = CacheHandlerUtil.getCacheHandler(cacheStrategy, localCacheConfig, redisCacheConfig);
+        this.cacheHandler = cacheHandler;
     }
 
     private static int calculateDaysRotation(int daysRotationParam) {
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheConfig.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheConfig.java
new file mode 100644
index 00000000..eceed709
--- /dev/null
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheConfig.java
@@ -0,0 +1,34 @@
+package com.datorama.oss.timbermill.common.cache;
+
+import com.datorama.oss.timbermill.common.redis.RedisService;
+
+public class CacheConfig {
+    private RedisService redisService;
+    private int cacheRedisTtlInSeconds;
+    private long maximumTasksCacheWeight;
+    private long maximumOrphansCacheWeight;
+
+    public CacheConfig(RedisService redisService, int cacheRedisTtlInSeconds, long maximumTasksCacheWeight, long maximumOrphansCacheWeight) {
+        this.redisService = redisService;
+        this.cacheRedisTtlInSeconds = cacheRedisTtlInSeconds;
+        this.maximumTasksCacheWeight = maximumTasksCacheWeight;
+        this.maximumOrphansCacheWeight = maximumOrphansCacheWeight;
+    }
+
+    RedisService getRedisService() {
+        return redisService;
+    }
+
+    int getCacheRedisTtlInSeconds() {
+        return cacheRedisTtlInSeconds;
+    }
+
+    long getMaximumTasksCacheWeight() {
+        return maximumTasksCacheWeight;
+    }
+
+    long getMaximumOrphansCacheWeight() {
+        return maximumOrphansCacheWeight;
+    }
+
+}
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheHandlerUtil.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheHandlerUtil.java
index 802a2879..40fd88a4 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheHandlerUtil.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/CacheHandlerUtil.java
@@ -1,16 +1,12 @@
 package com.datorama.oss.timbermill.common.cache;
 
-import com.datorama.oss.timbermill.LocalCacheConfig;
-import com.datorama.oss.timbermill.RedisCacheConfig;
-
 public class CacheHandlerUtil {
-    public static AbstractCacheHandler getCacheHandler(String strategy, LocalCacheConfig localCacheConfig, RedisCacheConfig redisCacheConfig) {
-
+    public static AbstractCacheHandler getCacheHandler(String strategy, CacheConfig cacheParams) {
         if (strategy.compareToIgnoreCase("redis") == 0){
-            return new RedisCacheHandler(redisCacheConfig);
+            return new RedisCacheHandler(cacheParams.getRedisService(), cacheParams.getCacheRedisTtlInSeconds());
         }
         else {
-            return new LocalCacheHandler(localCacheConfig);
+            return new LocalCacheHandler(cacheParams.getMaximumTasksCacheWeight(), cacheParams.getMaximumOrphansCacheWeight());
         }
     }
 }
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/LocalCacheHandler.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/LocalCacheHandler.java
index 785cbd6c..56bebd03 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/LocalCacheHandler.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/LocalCacheHandler.java
@@ -1,6 +1,5 @@
 package com.datorama.oss.timbermill.common.cache;
 
-import com.datorama.oss.timbermill.LocalCacheConfig;
 import com.datorama.oss.timbermill.common.KamonConstants;
 import com.datorama.oss.timbermill.unit.LocalTask;
 import com.google.common.cache.Cache;
@@ -18,9 +17,9 @@ public class LocalCacheHandler extends AbstractCacheHandler {
     private Cache<String, String> tasksCache;
     private Cache<String, List<String>> orphansCache;
 
-    LocalCacheHandler(LocalCacheConfig localCacheConfig) {
+    LocalCacheHandler(long maximumTasksCacheWeight, long maximumOrphansCacheWeight) {
         tasksCache = CacheBuilder.newBuilder()
-                .maximumWeight(localCacheConfig.getMaximumTasksCacheWeight())
+                .maximumWeight(maximumTasksCacheWeight)
                 .weigher((Weigher<String, String>) (key, value) -> 2 * (key.length() + value.length()))
                 .removalListener(notification -> {
                     String key = notification.getKey();
@@ -31,7 +30,7 @@ public class LocalCacheHandler extends AbstractCacheHandler {
                 .build();
 
         orphansCache = CacheBuilder.newBuilder()
-                .maximumWeight(localCacheConfig.getMaximumOrphansCacheWeight())
+                .maximumWeight(maximumOrphansCacheWeight)
                 .weigher(this::getEntryLength)
                 .removalListener(notification -> {
                     int entryLength = getEntryLength(notification.getKey(), notification.getValue());
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/RedisCacheHandler.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/RedisCacheHandler.java
index 18cd8f4e..ac9da406 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/RedisCacheHandler.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/cache/RedisCacheHandler.java
@@ -1,6 +1,5 @@
 package com.datorama.oss.timbermill.common.cache;
 
-import com.datorama.oss.timbermill.RedisCacheConfig;
 import com.datorama.oss.timbermill.common.redis.RedisService;
 import com.datorama.oss.timbermill.unit.LocalTask;
 import com.github.jedis.lock.JedisLock;
@@ -25,9 +24,12 @@ public class RedisCacheHandler extends AbstractCacheHandler {
     private final int redisTtlInSeconds;
 
 
-    RedisCacheHandler(RedisCacheConfig redisCacheConfig) {
-        this.redisService = redisCacheConfig.getRedisService();
-        this.redisTtlInSeconds = redisCacheConfig.getRedisTtlInSeconds();
+    RedisCacheHandler(RedisService redisService, int cacheRedisTtlInSeconds) {
+        if (redisService == null){
+            throw new RuntimeException("Redis cache used but no redis host defined");
+        }
+        this.redisService = redisService;
+        this.redisTtlInSeconds = cacheRedisTtlInSeconds;
     }
 
     @Override
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/PersistenceHandlerUtil.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/PersistenceHandlerUtil.java
index f56662c3..7c1e3594 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/PersistenceHandlerUtil.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/PersistenceHandlerUtil.java
@@ -6,11 +6,11 @@
 
 public class PersistenceHandlerUtil {
 
-	public static PersistenceHandler getPersistenceHandler(String persistenceHandlerStrategy,
+	public static PersistenceHandler getPersistenceHandler(String persistenceStrategy,
 														   Map<String, Object> params) {
 		PersistenceHandler persistenceHandler = null;
-		if (persistenceHandlerStrategy != null && !persistenceHandlerStrategy.toLowerCase().equals("none")){
-			persistenceHandler = getPersistenceHandlerByStrategy(persistenceHandlerStrategy, params);
+		if (persistenceStrategy != null && !persistenceStrategy.toLowerCase().equals("none")){
+			persistenceHandler = getPersistenceHandlerByStrategy(persistenceStrategy, params);
 			if (!persistenceHandler.isCreatedSuccessfully()){
 				persistenceHandler = null;
 			}
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/RedisPersistenceHandler.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/RedisPersistenceHandler.java
index 37a9617d..5ca8bf28 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/RedisPersistenceHandler.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/common/persistence/RedisPersistenceHandler.java
@@ -36,6 +36,9 @@ public class RedisPersistenceHandler extends PersistenceHandler {
         super(maxFetchedBulks, maxFetchedEvents, maxInsertTries);
         this.minLifetime = minLifetime;
         this.ttl = ttl;
+        if (redisService == null){
+            throw new RuntimeException("Redis persistence used but no redis host defined");
+        }
         this.redisService = redisService;
         LOG.info("Redis persistence handler is up.");
     }
diff --git a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/pipe/LocalOutputPipe.java b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/pipe/LocalOutputPipe.java
index 03ef5c8b..d70b31df 100644
--- a/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/pipe/LocalOutputPipe.java
+++ b/timbermill-java/timbermill-local/src/main/java/com/datorama/oss/timbermill/pipe/LocalOutputPipe.java
@@ -3,6 +3,9 @@
 import com.datorama.oss.timbermill.*;
 import com.datorama.oss.timbermill.common.ElasticsearchUtil;
 import com.datorama.oss.timbermill.common.KamonConstants;
+import com.datorama.oss.timbermill.common.cache.AbstractCacheHandler;
+import com.datorama.oss.timbermill.common.cache.CacheConfig;
+import com.datorama.oss.timbermill.common.cache.CacheHandlerUtil;
 import com.datorama.oss.timbermill.common.persistence.PersistenceHandler;
 import com.datorama.oss.timbermill.common.persistence.PersistenceHandlerUtil;
 import com.datorama.oss.timbermill.common.redis.RedisService;
@@ -43,11 +46,9 @@ private LocalOutputPipe(Builder builder) {
                 builder.numberOfShards, builder.numberOfReplicas, builder.maxTotalFields, builder.bulker, builder.scrollLimitation, builder.scrollTimeoutSeconds, builder.fetchByIdsPartitions,
                 builder.expiredMaxIndicesToDeleteInParallel);
 
-        LocalCacheConfig localCacheConfig = new LocalCacheConfig(builder.maximumTasksCacheWeight, builder.maximumOrphansCacheWeight);
-        RedisCacheConfig redisCacheConfig = new RedisCacheConfig(redisService, builder.redisTtlInSeconds);
-        taskIndexer = new TaskIndexer(builder.pluginsJson, builder.daysRotation, esClient, builder.timbermillVersion,
-                localCacheConfig, builder.cacheStrategy,
-                redisCacheConfig);
+        CacheConfig cacheParams = new CacheConfig(redisService, builder.redisTtlInSeconds, builder.maximumTasksCacheWeight, builder.maximumOrphansCacheWeight);
+        AbstractCacheHandler cacheHandler = CacheHandlerUtil.getCacheHandler(builder.cacheStrategy, cacheParams);
+        taskIndexer = new TaskIndexer(builder.pluginsJson, builder.daysRotation, esClient, builder.timbermillVersion,cacheHandler);
         cronsRunner = new CronsRunner();
         cronsRunner.runCrons(builder.bulkPersistentFetchCronExp, builder.eventsPersistentFetchCronExp, persistenceHandler, esClient,
                 builder.deletionCronExp, buffer, overflowedQueue,
diff --git a/timbermill-java/timbermill-server/src/main/java/com/datorama/timbermill/server/service/TimbermillService.java b/timbermill-java/timbermill-server/src/main/java/com/datorama/timbermill/server/service/TimbermillService.java
index 91e31b73..5654fcdd 100644
--- a/timbermill-java/timbermill-server/src/main/java/com/datorama/timbermill/server/service/TimbermillService.java
+++ b/timbermill-java/timbermill-server/src/main/java/com/datorama/timbermill/server/service/TimbermillService.java
@@ -1,16 +1,18 @@
 package com.datorama.timbermill.server.service;
 
 import com.datorama.oss.timbermill.ElasticsearchClient;
-import com.datorama.oss.timbermill.LocalCacheConfig;
-import com.datorama.oss.timbermill.RedisCacheConfig;
 import com.datorama.oss.timbermill.TaskIndexer;
 import com.datorama.oss.timbermill.common.ElasticsearchUtil;
+import com.datorama.oss.timbermill.common.cache.AbstractCacheHandler;
+import com.datorama.oss.timbermill.common.cache.CacheConfig;
+import com.datorama.oss.timbermill.common.cache.CacheHandlerUtil;
 import com.datorama.oss.timbermill.common.persistence.PersistenceHandler;
 import com.datorama.oss.timbermill.common.persistence.PersistenceHandlerUtil;
 import com.datorama.oss.timbermill.common.redis.RedisService;
 import com.datorama.oss.timbermill.cron.CronsRunner;
 import com.datorama.oss.timbermill.pipe.LocalOutputPipe;
 import com.datorama.oss.timbermill.unit.Event;
+import org.apache.commons.lang3.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -36,7 +38,7 @@ public class TimbermillService {
 	private boolean stoppedRunning = false;
 	private long terminationTimeout;
 	private PersistenceHandler persistenceHandler;
-	private CronsRunner cronsRunner;
+	private CronsRunner cronsRunner = new CronsRunner();
 
 	@Autowired
 	public TimbermillService(@Value("${INDEX_BULK_SIZE:200000}") Integer indexBulkSize,
@@ -63,7 +65,7 @@ public TimbermillService(@Value("${INDEX_BULK_SIZE:200000}") Integer indexBulkSi
 							 @Value("${MERGING_CRON_EXPRESSION:0 0/10 * 1/1 * ? *}") String mergingCronExp,
 							 @Value("${DELETION_CRON_EXPRESSION:0 0 12 1/1 * ? *}") String deletionCronExp,
 							 @Value("${DELETION_CRON_MAX_INDICES_IN_PARALLEL:1}") int expiredMaxIndicesToDeleteInParallel,
-							 @Value("${PERSISTENCE_HANDLER_STRATEGY:redis}") String persistenceHandlerStrategy,
+							 @Value("${PERSISTENCE_STRATEGY:sqlite}") String persistenceStrategy,
 							 @Value("${BULK_PERSISTENT_FETCH_CRON_EXPRESSION:0 0/10 * 1/1 * ? *}") String bulkPersistentFetchCronExp,
 							 @Value("${EVENTS_PERSISTENT_FETCH_CRON_EXPRESSION:0 0/5 * 1/1 * ? *}") String eventsPersistentFetchCronExp,
 							 @Value("${MAX_FETCHED_BULKS_IN_ONE_TIME:10}") int maxFetchedBulksInOneTime,
@@ -80,7 +82,7 @@ public TimbermillService(@Value("${INDEX_BULK_SIZE:200000}") Integer indexBulkSi
 							 @Value("${CACHE_TTL_IN_SECONDS:604800}") int cacheRedisTtlInSeconds,
 							 @Value("${REDIS_MAX_MEMORY:}") String redisMaxMemory,
 							 @Value("${REDIS_MAX_MEMORY_POLICY:}") String redisMaxMemoryPolicy,
-							 @Value("${REDIS_HOST:localhost}") String redisHost,
+							 @Value("${REDIS_HOST:}") String redisHost,
 							 @Value("${REDIS_PORT:6379}") int redisPort,
 							 @Value("${REDIS_PASS:}") String redisPass,
 							 @Value("${REDIS_USE_SSL:false}") Boolean redisUseSsl,
@@ -95,22 +97,22 @@ public TimbermillService(@Value("${INDEX_BULK_SIZE:200000}") Integer indexBulkSi
 		overflowedQueue = new LinkedBlockingQueue<>(overFlowedQueueCapacity);
 		terminationTimeout = terminationTimeoutSeconds * 1000;
 
-
-		RedisService redisService = new RedisService(redisHost, redisPort, redisPass, redisMaxMemory,
-				redisMaxMemoryPolicy, redisUseSsl, redisGetSize, redisPoolMinIdle, redisPoolMaxIdle,
-				redisPoolMaxTotal, redisMaxTries);
+		RedisService redisService = null;
+		if (!StringUtils.isEmpty(redisHost)) {
+			redisService = new RedisService(redisHost, redisPort, redisPass, redisMaxMemory,
+					redisMaxMemoryPolicy, redisUseSsl, redisGetSize, redisPoolMinIdle, redisPoolMaxIdle,
+					redisPoolMaxTotal, redisMaxTries);
+		}
 		Map<String, Object> params = PersistenceHandler.buildPersistenceHandlerParams(maxFetchedBulksInOneTime, maxOverflowedEventsInOneTime, maxInsertTries, locationInDisk, minLifetime, persistenceRedisTtlInSec, redisService);
-		persistenceHandler = PersistenceHandlerUtil.getPersistenceHandler(persistenceHandlerStrategy, params);
+		persistenceHandler = PersistenceHandlerUtil.getPersistenceHandler(persistenceStrategy, params);
 
 		ElasticsearchClient es = new ElasticsearchClient(elasticUrl, indexBulkSize, indexingThreads, awsRegion, elasticUser,
 				elasticPassword, maxIndexAge, maxIndexSizeInGB, maxIndexDocs, numOfElasticSearchActionsTries, maxBulkIndexFetches, searchMaxSize, persistenceHandler, numberOfShards, numberOfReplicas,
 				maxTotalFields, null, scrollLimitation, scrollTimeoutSeconds, fetchByIdsPartitions, expiredMaxIndicesToDeleteInParallel);
-		LocalCacheConfig localCacheConfig = new LocalCacheConfig(maximumTasksCacheWeight, maximumOrphansCacheWeight);
-		RedisCacheConfig redisCacheConfig= new RedisCacheConfig(redisService, cacheRedisTtlInSeconds);
-		taskIndexer = new TaskIndexer(pluginsJson, daysRotation, es, timbermillVersion,
-				localCacheConfig, cacheStrategy,
-				redisCacheConfig);
-		cronsRunner = new CronsRunner();
+
+		CacheConfig cacheParams = new CacheConfig(redisService, cacheRedisTtlInSeconds, maximumTasksCacheWeight, maximumOrphansCacheWeight);
+		AbstractCacheHandler cacheHandler = CacheHandlerUtil.getCacheHandler(cacheStrategy, cacheParams);
+		taskIndexer = new TaskIndexer(pluginsJson, daysRotation, es, timbermillVersion, cacheHandler);
 		cronsRunner.runCrons(bulkPersistentFetchCronExp, eventsPersistentFetchCronExp, persistenceHandler, es, deletionCronExp,
 				eventsQueue, overflowedQueue, mergingCronExp);
 		startQueueSpillerThread();


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index a9e9183a..63bbbcb1 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -41,6 +41,7 @@ jobs:
       LOCATION_IN_DISK: ./
       CACHE_STRATEGY: redis
       REDIS_HOST: localhost
+      PERSISTENCE_STRATEGY: redis
     steps:    
     - name: Cache Maven packages
       uses: actions/cache@v2