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.

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

ConfigProvider#orElse does not replace value from default provider #1186

Closed
dobrynya opened this issue Jun 8, 2023 · 5 comments
Closed

ConfigProvider#orElse does not replace value from default provider #1186

dobrynya opened this issue Jun 8, 2023 · 5 comments

Comments

@dobrynya
Copy link

dobrynya commented Jun 8, 2023

I use zio-config-typesafe 4.0.0-RC16 and zio 2.0.15

import zio.*
import zio.test.*, TestAspect.withLiveConsole
import zio.ConfigProvider
import zio.config.magnolia.deriveConfig
import zio.test.ZIOSpecDefault
import zio.config.typesafe._

case class A(b: List[String])

case object A {
  val descriptor = deriveConfig[A]
}

object ZioConfigTests extends ZIOSpecDefault:
  val config = ConfigProvider.fromHoconString(
    """
      |b = ["a", "b", "c"]
      |""".stripMargin
  )

  val config2 = ConfigProvider.fromHoconString(
    """
      |b = "a,b,c"
      |""".stripMargin, true
  )

  override def spec: Spec[TestEnvironment with Scope, Any] =
    suite("ZIO.config test suite")(
      test("Environment variable should replace configuration with a list literal") {
        for {
          _ <- TestSystem.putEnv("B", "Replaced value 1,Replaced value 2")
          c <- ZIO.config(A.descriptor)
          _ <- Console.printLine(c)
        } yield assertTrue(c.b == List("Replaced value 1", "Replaced value 2"))
      }.provideLayer(Runtime.setConfigProvider(ConfigProvider.defaultProvider orElse config))
      ,
      test("Environment variable should replace configuration with a list separated by commas") {
        for {
          _ <- TestSystem.putEnv("B", "Replaced value 1,Replaced value 2")
          c <- ZIO.config(A.descriptor)
          _ <- Console.printLine(c)
        } yield assertTrue(c.b == List("Replaced value 1", "Replaced value 2"))
      }.provideLayer(Runtime.setConfigProvider(ConfigProvider.defaultProvider orElse config2))
    ) @@ withLiveConsole

Expected the field b should be replaced by the environment variable and be equal List("Replaced value 1", "Replaced value 2"). Actually the value is not replaced when a hocun string contains a aproperty defined as an array.

@przemek-pokrywka
Copy link

@dobrynya, why do you expect that the value of b gets replaced in this case, I'm wondering if I'm missing something. I think that by default keys from HOCON files are not equivalent to environment variables (nor system properties) unless one uses some special setting, which I can't recall at the moment.

On the other hand, the environment variables from TestSystem seem to be ignored entirely, even if one tries to access them properly (with ${VARIABLE_NAME} syntax from within the HOCON file).

@dobrynya
Copy link
Author

@przemek-pokrywka I consider that a HOCON file, system environment, JVM props are configuration sources with slightly different notations, dispite difference in syntax. So it's expected that one can completely overwrite properties of lower priority when values are specified in a source of higher priority.
My use case was as follows:

  • I created a HOCON file with default configuration
  • In runtime I tried reconfiguring my application with values in system environment
  • zio-config ignored values in sys env, so application was unable to run

@przemek-pokrywka
Copy link

@dobrynya your problem is that you use Typesafe/Lightbend Config library, as it is hardcoded to always use the real system environment variables from the java.lang.System class. So even the zio-config wrapper around it is not able to get around this limitation, at least without resorting to some ugly and fragile (JVM-version-specific) reflection hacks.

I've proposed a PR that lets one increase the testability of Typesafe/Lightbend Config, but the maintainers decided that they will not merge it.

If you want, you may try to persuade them that they change their minds.

@przemek-pokrywka
Copy link

@dobrynya The mentioned PR: lightbend/config#797, I'd appreciate it if you backed it up.

@guersam
Copy link
Contributor

guersam commented Jan 4, 2024

@dobrynya @przemek-pokrywka Maybe it's caused by this one: zio/zio#8604

@dobrynya dobrynya closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants