Ido Barkan has posted comments on this change. Change subject: Get list of nameservers for host ......................................................................
Patch Set 2: Code-Review-1 (6 comments) https://gerrit.ovirt.org/#/c/39460/2//COMMIT_MSG Commit Message: Line 6: Line 7: Get list of nameservers for host Line 8: Line 9: 1. Get a list of nameservers listed in /etc/resolv.conf Line 10: 2. Perform unit testing for above functionality it is hopefully obvious for everyone, so you can drop this line Line 11: Line 12: Change-Id: I3ce8a0e2bf93b118620739b144e70c576f03cd08 https://gerrit.ovirt.org/#/c/39460/2/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 90: try: Line 91: with open(DNS_RESOLVE_CONF_FILE,'r') as file_object: Line 92: file_text = file_object.read() Line 93: nameservers_list = parse_dnss(file_text) Line 94: return nameservers_list this can be in-lined since the expected result is already documented Line 95: except IOError : Line 96: logging.error('Unable to open file /etc/resolv.conf') Line 97: return 0 Line 98: Line 91: with open(DNS_RESOLVE_CONF_FILE,'r') as file_object: Line 92: file_text = file_object.read() Line 93: nameservers_list = parse_dnss(file_text) Line 94: return nameservers_list Line 95: except IOError : since this function returns a list, let's be consistent so we would not crash the caller. just return an empty list. in fact, I wouldn't even try/except. /etc/resolve.conf is not expected to be missing or not readable in any supported Linux distribution. I would expet the system to explode in case an exception is raised here. Line 96: logging.error('Unable to open file /etc/resolv.conf') Line 97: return 0 Line 98: Line 99: Line 96: logging.error('Unable to open file /etc/resolv.conf') Line 97: return 0 Line 98: Line 99: Line 100: def parse_dnss(file_text): should be python 'private' (prefixed with '_') Line 101: dnss = [] Line 102: for line in file_text.splitlines(): Line 103: line.strip() Line 104: words = line.split() Line 99: Line 100: def parse_dnss(file_text): Line 101: dnss = [] Line 102: for line in file_text.splitlines(): Line 103: line.strip() this statement has no affect. strings are immutable in python hence all string methods return a copy of the original string. this will do what you want: words = line.strip().split() Line 104: words = line.split() Line 105: if words[0] == 'nameserver': Line 106: dnss.append(words[1]) Line 107: return dnss Line 946: if iface == vdict['iface']: Line 947: users.add(v) Line 948: return users Line 949: Line 950: please remove this new blank line. it is unrelated to this commit. Line 951: def ifaceUsed(iface): Line 952: """Lightweight implementation of bool(Netinfo.ifaceUsers()) that does not Line 953: require a NetInfo object.""" Line 954: if os.path.exists(os.path.join(NET_PATH, iface, 'brport')): # Is it a port -- To view, visit https://gerrit.ovirt.org/39460 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3ce8a0e2bf93b118620739b144e70c576f03cd08 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Priya Avhad <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Priya Avhad <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
