-
Notifications
You must be signed in to change notification settings - Fork 705
Conversation
rnpridgeon
commented
Jun 30, 2016
- Add repository at base level
- kafka container extends zookeeper (install kafka 1 time)
- Make configure, ensure and launch scripts more generic
TODO: Add unit tests, propose a few changes to base image to take advantage of UNION FS layering
1. Add repository at base level 2. kafka container extends zookeeper (install kafka 1 time) 3. Make configure, ensure and launch scripts more generic
echo "===> Printing /etc/${COMPONENT}/${COMPONENT}.properties " | ||
cat /etc/${COMPONENT}/${COMPONENT}.properties | ||
|
||
echo "===> Launching kafka ... " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
launching "rest proxy"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, should be ${COMPONENT}. Good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
echo "===> Printing /etc/kafka/server.properties " | ||
cat /etc/kafka/server.properties | ||
echo "===> Printing /etc/${COMPONENT}/${COMPONENT}.properties " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jeremy suggested removing this line as this has SSL password and it would be a security bug.
|
||
echo "===> Launching ${COMPONENT} ... " | ||
# Don't need exec as the command has it. | ||
${COMPONENT}-start /etc/${COMPONENT}/${COMPONENT}.properties |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still do need to add 'exec' here per @theduderog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nevermind. I'm going to go ahead and merge this PR and then I'll make this change when I add Schema Registry.