alexisdacquay / does_it_live Goto Github PK

Hey Alex,
have gone through your code. looks good. here are a few suggestions for improvements

1. Run your script through PEP8 checker and pylint. You have a few unconventional things that look out of place (for example, extra spaces around variables, lower case class names, capital global vars)
2. You can replace your trace/trace2 with built-in logging module
3. Unless you really need to, I'd drop support for python2. it goes EOL in a few years anyway
4. Not sure why you need global args at the start of your main. If you functions/classes need access to args, just pass it to them as variables. use globals only for some system-wide defaults that are unlikely to change much
5. Add the except KeyboardInterrupt to your while True to gracefully exit your script
6. it's generally bad practice to return different set of values from the same method call, like what you do with check.isAlive(). ideally, i'd make dns and icmp children of a parent class Check. you can set some common variables like host, source in the parent class and check-specific in child classes. similar to what i've done in docker-topo. I've got Device class and CEOS and Host classes inheriting from it. Just make things easier (at least for me) and maximises code re-use. You can decide if it makes sense for you or not.

HTH, ping me if you have any specific questions