Parsing of JSON settings could be simplified

[ceilican]

I would suggest using play.api.libs.json.Json and making Settings a case class. Similar to what is done in Agora for parsing parameters (see https://gitlab.com/aossie/Agora/blob/master/src/main/scala/agora/parser/ParameterParser.scala and lines 39--65 in https://gitlab.com/aossie/Agora/blob/master/src/main/scala/agora/model/Parameters.scala).

[atapin]

@ceilican I'd suggest using circe. It's a more lightweight and advanced lib.

[ceilican]

@atapin, Thanks for the comment.

My main point is that Settings seems to parse JSON files (using circe) in a way that is more complicated than it needs to be. I know that it would be possible to do it in a simpler way with Play's Json API, as it is done in Agora. I am not as familiarized with circe, but I guess it would be possible to make it simpler with circe as well.

So, the main question is: is there a reason to keep Settings as it is? Or would a refactoring be welcome?

[RicoGit]

Why don't you keep settings in typesafe's HOCON?

[Tolsi]

In the near future, the configuration of the Scorex project will go to HOCON

[ceilican]

@Tolsi , I added a new field to NetworkSettings and Ficus is not managing to automatically obtain the value for that field from "settings.conf". My changes and the stack trace can be seen here: 60713a8 .

If you have some time, could you have a look and tell me what else I need to do?

Also, I noticed that the example "settings.conf" files don't have values for all the fields in the NetworkSettings case class. Where does Ficus obtain default values?

[ceilican]

Also, I noticed that the example "settings.conf" files don't have values for all the fields in the NetworkSettings case class. Where does Ficus obtain default values?

Ok. After debugging the config parsing process, I got the answer to the question above. The parser falls back on “src/main/resources/reference.conf”. So, values that are not specified in “examples/src/test/resources/settings.conf” are read from “src/main/resources/reference.conf”.

I still don't know the answer to the first question (i.e. why Ficus is failing to convert the config to a NetworkSettings object, despite the changes I made in 60713a8), though... Does anyone know? (I still haven't debugged this part of the process yet...)

[ceilican]

In this commit I show that, if I add deliveryTimeout directly in ScorexSettings, Ficus works. But if I add it in NetworkSettings, it doesn't. I also tried to add it in one of the other nested case classes (WalletSettings), and it also didn't work.

I still don't know what is causing this problem. For a short while I thought it could be Scala's limits on the number of arguments for case classes, or some other similar limit. I don't think this is the problem now, but it could become a problem in the future, if Scorex's configuration case classes grow too large.

[ceilican]

Ok. Problem solved. I documented this strange behaviour here: https://stackoverflow.com/questions/47068594/why-is-ficus-throwing-an-exception-when-i-try-to-add-a-new-field-in-my-case-clas/47071839#47071839 . And I notified the Ficus developers of this potential bug here: iheartradio/ficus#55 .

[ceilican]

@kushti @catena2w this issue can be closed.