Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing host resource and provider out-of-sync #174

Merged
merged 1 commit into from
Jul 17, 2014

Conversation

nightw
Copy link
Contributor

@nightw nightw commented Jul 9, 2014

From #173

The resources/host.rb file contains attributes for groups, templates and interfaces, but those are currently ignored in the providers/host.rb, since that file is only using the parameters[:groupNames], parameteres[:templates] and parameteres[:interfaces] attribute respectively for registering a host to the Zabbix server.

Fixing this issue while I tried to keep the backward compatibility to not brake the agent_registration recipe and other people's application cookbooks using this resource the old way. Could you please review it?

…on. Also kept backward compatibility where it was possible.
@nightw nightw changed the title Fixing issue #173 Fixing host resource and provider out-of-sync Jul 9, 2014
@nightw
Copy link
Contributor Author

nightw commented Jul 9, 2014

It fixes issue #173

@@ -19,8 +19,6 @@
# for appropriate inventory hash keys (Property name from the table)
attribute :inventory, :kind_of => Hash, :default => {}

attribute :interfaces, :kind_of => Array, :default => []
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you removing interfaces?
You are using it here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a sneak-in small fix for this line to being a duplicate in the resource file (for some time, I've not checked the git blame for this too much). The other, same line is preserved, see here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I like to see small fix like that! :)
That mean that some people read the code.

@guilhem
Copy link
Collaborator

guilhem commented Jul 10, 2014

Seems to be ok for me (modulo my inline comments)

@guilhem
Copy link
Collaborator

guilhem commented Jul 10, 2014

👍 for me if @nightw can guarantee us that TK is OK ;)

@nightw
Copy link
Contributor Author

nightw commented Jul 10, 2014

What does 'TK' mean? :)

@guilhem
Copy link
Collaborator

guilhem commented Jul 10, 2014

Oh sorry, it stand for Test Kitchen

@nightw
Copy link
Contributor Author

nightw commented Jul 10, 2014

Oh, my bad, I'm using Test Kitchen myself too, I just did not know the acronym. I've just tested it and it's OK (the opscode-centos-5.9 box is 404, the others were good). (btw this is not surprising, since the .kitchen.yml file only tests the default recipe, which only installs the Zabbix agent. I think the coverage should be increased somehow.)

Also Foodcritic does not tell any hints.

@nightw
Copy link
Contributor Author

nightw commented Jul 17, 2014

Since the TK is also OK, please merge this

guilhem added a commit that referenced this pull request Jul 17, 2014
Fixing host resource and provider out-of-sync
@guilhem guilhem merged commit 0529156 into laradji:master Jul 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants