cerner / cerner_kafka

A Kafka Cookbook for Chef
Apache License 2.0
30 stars 25 forks source link

Provided configuration for controlling offset monitor logging #52

Closed bbaugher closed 8 years ago

noslowerdna commented 8 years ago

+1

bbaugher commented 8 years ago

In order to get everything to work I had to include an option to download and supply the log4j implementation jar. In later version of the offset monitor (not released) log4j jar is included with the offset monitor jar

bbaugher commented 8 years ago

Not sure why travis says the builds failed as you can see all the tests pass

gwhitsitt commented 8 years ago

+1

hercules90 commented 8 years ago

+1 after the minor comment.

bbaugher commented 8 years ago

Ended up making a couple other changes after realizing what might happen to our existing installations.

Previously the classpath for the offset monitor referenced the jar itself and we would land the jar with the version in the name. The change here is to include the entire offset monitor install directory on the classpath to have it include our log4j config and possibly log4j jar. However this would mean if you updated to a new version of the offset monitor there would be 2 offset monitor jars in the install directory / classpath since the file name would contain the version. The change then is to keep the offset monitor jar file name the same and leave it up to Chef to update it appropriately.

The next problem is older installations (prior to this change) would have the same problem 2 offset monitor jars on the classpath. The simplest fix for this I could come up with was to change the default value for the offset monitor installation directory.

Let me know if you have any problems with this plan. Theres other ways to get around these issues but this seemed pretty easy.

noslowerdna commented 8 years ago

+1

dhondgepooja commented 8 years ago

+1

bbaugher commented 8 years ago

Squashed. Tests passed, merging