WW-Digital / reactive-kinesis

A Scala & Akka based implementation for working with Amazon Kinesis Streams
Apache License 2.0
34 stars 13 forks source link

KPL Typed Config #65

Closed etspaceman closed 6 years ago

etspaceman commented 6 years ago

Solves #22

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.7%) to 90.173% when pulling 56a7d745de1bd45ac044a8a655a18639b0afa9ab on etspaceman:typed-config into 3bd04e5aaac03aa22377278011eb24ab416d1d1f on WW-Digital:master.

etspaceman commented 6 years ago

@markglh this is ready for review

etspaceman commented 6 years ago

Hey Mark -

Yeah these seem reasonable. I bet I could derive the defaults from the existing typesafe-config loader rather than hardcoding them; it felt awkward to me as well.

RE your 3rd point, I'll have to think about that and an approach for it. It would make sense for the test to also check for new values against the typed config, so I agree that it would be a valuable addition.

I might have some time to work on this in the next couple of days, I'll let you know when I have something for you to review.

etspaceman commented 6 years ago

@markglh - Just pushed another try. Let me know what you think

markglh commented 6 years ago

Thanks @etspaceman, looks good! Just need to fix the Jenkins Travis error (default param issue?):

[error] /home/travis/build/WW-Digital/reactive-kinesis/src/main/scala/com/weightwatchers/reactive/kinesis/producer/ProducerConf.scala:31:8: in object ProducerConf, multiple overloaded alternatives of method apply define default arguments.
[error] object ProducerConf {
[error]        ^
[error] one error found
etspaceman commented 6 years ago

Hrm, odd. Compiled fine on my end. Looking into it

etspaceman commented 6 years ago

@markglh I fixed the compile error but now we're getting docker-compose errors on the Travis build?

markglh commented 6 years ago

waaat - will look into it

markglh commented 6 years ago

Ok so travis changed something (it no longer automatically falls back to the sudo env automatically). docker & docker-compose are ancient versions in the container env so I've forced it to use the trusty sudo one (slower but more powerful).

try replacing your travis.yml with this: https://github.com/WW-Digital/reactive-kinesis/blob/test-build/.travis.yml

We can update it in this Pr instead of raising a new one :)