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

case class default values are non-lazy #1218

Closed
dontgitit opened this issue Nov 23, 2021 · 6 comments
Closed

case class default values are non-lazy #1218

dontgitit opened this issue Nov 23, 2021 · 6 comments

Comments

@dontgitit
Copy link

dontgitit commented Nov 23, 2021

import pureconfig._
import pureconfig.generic.auto._

case class SampleConf(foo: String = ???)

ConfigSource.string("{ foo: bar }").load[SampleConf]

the above produces:

scala.NotImplementedError: an implementation is missing
  at scala.Predef$.$qmark$qmark$qmark(Predef.scala:347)
  at SampleConf$.apply$default$1(<console>:1)
  at anon$exportedReader$macro$16$1.inst$macro$1$lzycompute(<console>:1)
  at anon$exportedReader$macro$16$1.inst$macro$1(<console>:1)
  ... 35 elided

I would expect it to produce Right(SampleConf(bar)) since foo is defined. It seems the default values are being invoked even if they're unneeded. This example is a little egregious (using ???) but the default value might involve a complex operation that we don't want to be computed if it's unnecessary.

@dontgitit
Copy link
Author

P.S. It breaks even when it's nested inside a nullable field;

import pureconfig._
import pureconfig.generic.auto._

case class Foo(bar: String, baz: Option[String] = ???)

case class Blah(hello: String, world: Option[Foo])

ConfigSource.string("{ hello: hey }").load[Blah]

which again makes:

scala.NotImplementedError: an implementation is missing
  at scala.Predef$.$qmark$qmark$qmark(Predef.scala:347)
  at Foo$.apply$default$2(<console>:1)
  at anon$exportedReader$macro$99$1.inst$macro$49$lzycompute(<console>:1)
  at anon$exportedReader$macro$99$1.inst$macro$49(<console>:1)
  at anon$exportedReader$macro$99$1.$anonfun$inst$macro$21$1(<console>:1)
  at shapeless.Lazy$$anon$1.value$lzycompute(lazy.scala:123)
  at shapeless.Lazy$$anon$1.value(lazy.scala:123)
  at shapeless.lazily$.apply(lazy.scala:142)
  at anon$exportedReader$macro$99$1.inst$macro$21$lzycompute(<console>:1)
  at anon$exportedReader$macro$99$1.inst$macro$21(<console>:1)
  at anon$exportedReader$macro$99$1.$anonfun$inst$macro$20$1(<console>:1)
  at shapeless.Lazy$$anon$1.value$lzycompute(lazy.scala:123)
  at shapeless.Lazy$$anon$1.value(lazy.scala:123)
  at pureconfig.generic.MapShapedReader$$anon$2.fromWithDefault(MapShapedReader.scala:53)
  at pureconfig.generic.MapShapedReader$$anon$2.fromWithDefault(MapShapedReader.scala:47)
  at pureconfig.generic.MapShapedReader$$anon$2.fromWithDefault(MapShapedReader.scala:65)
  at pureconfig.generic.MapShapedReader$$anon$2.fromWithDefault(MapShapedReader.scala:47)
  at pureconfig.generic.DerivedConfigReader1$$anon$3.$anonfun$from$7(DerivedConfigReader.scala:70)
  at scala.util.Either$RightProjection.flatMap(Either.scala:757)
  at pureconfig.generic.DerivedConfigReader1$$anon$3.from(DerivedConfigReader.scala:70)
  at pureconfig.ConfigSource.$anonfun$load$1(ConfigSource.scala:67)
  at scala.util.Either$RightProjection.flatMap(Either.scala:757)
  at pureconfig.ConfigSource.load(ConfigSource.scala:67)
  at pureconfig.ConfigSource.load$(ConfigSource.scala:66)
  at pureconfig.ConfigObjectSource.load(ConfigSource.scala:91)
  ... 35 elided

I would expect this to never even try instantiating a Foo since Blah defines world: Option[Foo] and there's no world in the provided config, right?

@ruippeixotog
Copy link
Member

Hi @dontgitit, thanks for filing this issue. Indeed, it looks like we are instantiating defaults earlier than we need to and that might be a problem (though I wouldn't recommend setting any kind of heavy logic, particularly stateful logic, in the defaults of config classes).

I'm not sure we can avoid computing all defaults in all cases, but I'll look into it.

@dontgitit
Copy link
Author

I wouldn't recommend setting any kind of heavy logic, particularly stateful logic, in the defaults of config classes

Absolutely!
For context, I was considering having some default values loaded from EC2 instance tags or metadata API [in production, if config value missing] and was surprised when the code ran in dev [when the config was provided].

@ruippeixotog
Copy link
Member

Hi @dontgitit, a very late update on this: at the time I investigated the possibility of making default values lazy, but unfortunately it seems to be out of our control. We rely on shapeless's Default case class for this, which does not provide us enough control to compute defaults only on demand. Getting default implementations seem to be possible only using macros that rely on internal compiler implementation details, so I don't think we can replicate this on the PureConfig side either...

I guess my only recommendation would be really to use only simple side-effect free fields as defaults. With a bit of extra boilerplate, one can always achieve the same functionality and keep impure logic separate, e.g.:

case class MyConfig(myTag: Option[String]) {
  def myTagValue: String = myTagValue.getOrElse(getFromAWS())
}

@dontgitit
Copy link
Author

That makes sense. Thanks for taking a look!

@ruippeixotog
Copy link
Member

This was fixed in #1636 for Scala 3. We have no plans to back port this to Scala 2 due to shapeless limitations, so I'm going to close this as completed.

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

2 participants