Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update ConfigFileClient to allow an invalid path #22187

Merged
merged 1 commit into from
Feb 1, 2023
Merged

update ConfigFileClient to allow an invalid path #22187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 17 additions & 8 deletions airbyte-featureflag/src/main/kotlin/Client.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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) }
}
}
}
}
}
Expand All @@ -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)
}
}

/**
Expand Down
26 changes: 26 additions & 0 deletions airbyte-featureflag/src/test/kotlin/ClientTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`() {
Expand Down