search

dhruvbansal / rubix

A Ruby client for configuring and writing to Zabbix
Other
25 stars 12 forks source link

Support DNS even if use_ip is set. #5

Closed rvalente closed 12 years ago

rvalente commented 12 years ago

IP and DNS should be filled even if use_ip is set. I also added more validations to save a user from entering a less than intelligent host definition around IPs and DNS.

dhruvbansal commented 12 years ago

thanks for the patch! i'll take a look soon and give it a merge.

rvalente commented 12 years ago

You're welcome, let me know if you have any questions with my logic. I tried to keep the style as similar as possible. I am running Zabbix 1.8.13 in production. End of the day I like to see both IP and hostname in Zabbix.

unilogic commented 12 years ago

I agree with Ron on this one. I'd like to be able to set both IP and DNS no matter if use_ip is set or not. This also matches the behavior of the web interface.

rvalente commented 12 years ago

Thanks @unilogic, for the time being I have just compiled the gem myself and hosted it locally on a gem server.

unilogic commented 12 years ago

@dhruvbansal any updates on your plans on merging this code?

dhruvbansal commented 12 years ago

working on it right now!

dhruvbansal commented 12 years ago

sorry this is taking me so long to deal with, i'm traveling and can't presently get access to my machine where i run the integration tests. i did run it once previously and noticed that many of the specs failed. this was largely due to assumptions the specs make about what is a valid host that are invalid with this new patch. we use rubix in production at work all the time and the specs reflect stuff that we need to be stable so it's important to me that they pass.

if you can fix those specs so they pass, i'll accept the patch. otherwise we'll have to wait a day or two for me to get back home and be able to fix them myself.

let me know if it's unclear how to run the specs -- i'm afraid they're sort of clunky and not well-documented at present.

dhruvbansal commented 12 years ago

ok -- i fixed the specs myself (just required making sure i set the IP on all hosts i was creating during integration tests) and pushed out a new gem, v. 0.5.5.

thanks for the patch!