search

jaytaylor / ansible-kafka

Ansible Kafka role
https://galaxy.ansible.com/jaytaylor/kafka/
Other
67 stars 52 forks source link

Overwriteable kafka settings #37

Closed jordiclariana closed 6 years ago

jordiclariana commented 6 years ago

Improve kafka variables scopes and settings

Hi,

The main purpose of this PR is to be able to overwrite particular settings in Kafka server/producer without the need of rewrite the whole server and/or producer variable in your playbook or variables file.

It also "fixes" the variable scoping, renaming all non-starting with "kafka_" variables. Take a look at the modified README.md, which tried to explain the changes and new variables usage.

I also added a Version notes section, as I think that this change breaks with previous versions and is important to explain how. I assume next version is 1.4.0 (as current latest tag is 1.3.0), but maybe if the change is too "radical" it could be named 2.0.0.

Lastly I fixed some conditions that throw error when the role is run in check_mode.

I tried all these changes and it worked well in my environment (obviously having in account the variables renaming). I run it against an already provisioned kafka server using 1.3.0 role version.

jordiclariana commented 6 years ago

Travis fails for versions <2.2.x.x. Can i change Travis config to stop supporting those versions? There been surpassed by 2.2 for a long time now. I will include a warning/notice in the README.md too and change the meta info. Also, if we do that, then I guess that after this PR the version of the role should be changed to a new major: 2.0.0

What do you think?

jaytaylor commented 6 years ago

Hi Jordi,

This looks cool! Can you squash the commits in this PR in order to maintain a clean commit history once it is merged into master?

jordiclariana commented 6 years ago

There you go!

jordiclariana commented 6 years ago

I just pushed a fix for kafka_healthcheck_address fact not well set when kafka_server.host_name is set to "".

jaytaylor commented 6 years ago

LGTM, thank you @jordiclariana !