update ConfigFileClient to allow an invalid path (#22187)

allow ConfigFileClient to accept an invalid path now logs a messages instead of throwing an exception
airbytehq · Feb 1, 2023 · d36032f · d36032f
1 parent 96b70bc
commit d36032f
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 8 deletions.
diff --git a/airbyte-featureflag/src/main/kotlin/Client.kt b/airbyte-featureflag/src/main/kotlin/Client.kt
@@ -15,6 +15,7 @@ import com.launchdarkly.sdk.server.LDClient
 import io.micronaut.context.annotation.Property
 import io.micronaut.context.annotation.Requires
 import jakarta.inject.Singleton
+import org.slf4j.LoggerFactory
 import java.lang.Thread.MIN_PRIORITY
 import java.nio.file.Path
 import java.nio.file.StandardWatchEventKinds
@@ -24,6 +25,7 @@ import kotlin.concurrent.read
 import kotlin.concurrent.thread
 import kotlin.concurrent.write
 import kotlin.io.path.isRegularFile
+import kotlin.io.path.notExists
 
 /**
  * Feature-Flag Client interface.
@@ -60,19 +62,22 @@ internal const val CONFIG_FF_PATH = "airbyte.feature-flag.path"
 @Requires(property = CONFIG_FF_CLIENT, notEquals = CONFIG_FF_CLIENT_VAL_LAUNCHDARKLY)
 class ConfigFileClient(@Property(name = CONFIG_FF_PATH) config: Path?) : FeatureFlagClient {
     /** [flags] holds the mappings of the flag-name to the flag properties */
-    private var flags: Map<String, ConfigFileFlag> = config?.let { readConfig(it) } ?: mapOf()
+    private var flags: Map<String, ConfigFileFlag> = mapOf()
 
     /** lock is used for ensuring access to the flags map is handled correctly when the map is being updated. */
     private val lock = ReentrantReadWriteLock()
 
     init {
-        config?.also {
-            if (!it.isRegularFile()) {
-                throw IllegalArgumentException("config must reference a file")
-            }
-
-            it.onChange {
-                lock.write { flags = readConfig(config) }
+        config?.also { path ->
+            when {
+                path.notExists() -> log.info("path $path does not exist, will return default flag values")
+                !path.isRegularFile() -> log.info("path $path does not reference a file, will return default values")
+                else -> {
+                    flags = readConfig(path)
+                    path.onChange {
+                        lock.write { flags = readConfig(config) }
+                    }
+                }
             }
         }
     }
@@ -83,6 +88,10 @@ class ConfigFileClient(@Property(name = CONFIG_FF_PATH) config: Path?) : Feature
             else -> lock.read { flags[flag.key]?.enabled ?: flag.default }
         }
     }
+
+    companion object {
+        private val log = LoggerFactory.getLogger(ConfigFileClient::class.java)
+    }
 }
 
 /**

diff --git a/airbyte-featureflag/src/test/kotlin/ClientTest.kt b/airbyte-featureflag/src/test/kotlin/ClientTest.kt
@@ -67,6 +67,32 @@ class ConfigFileClientTest {
         }
     }
 
+    @Test
+    fun `verify missing file returns default flag state`() {
+        val client: FeatureFlagClient = ConfigFileClient(Path.of("src", "test", "resources", "feature-flags-dne-missing.yml"))
+        val defaultFalse = Temporary(key = "default-false")
+        val defaultTrue = Temporary(key = "default-true", default = true)
+
+        val ctx = Workspace("workspace")
+        with(client) {
+            assertTrue { enabled(defaultTrue, ctx) }
+            assertFalse { enabled(defaultFalse, ctx) }
+        }
+    }
+
+    @Test
+    fun `verify directory instead of file returns default flag state`() {
+        val client: FeatureFlagClient = ConfigFileClient(Path.of("src", "test", "resources"))
+        val defaultFalse = Temporary(key = "default-false")
+        val defaultTrue = Temporary(key = "default-true", default = true)
+
+        val ctx = Workspace("workspace")
+        with(client) {
+            assertTrue { enabled(defaultTrue, ctx) }
+            assertFalse { enabled(defaultFalse, ctx) }
+        }
+    }
+
     @Test
     @Ignore
     fun `verify config-file reload capabilities`() {