Change in vdsm[master]: Get list of nameservers for host
ibarkan at redhat.com
ibarkan at redhat.com
Sun Apr 12 10:27:27 UTC 2015
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 <avhad.priya24 at gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Priya Avhad <avhad.priya24 at gmail.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list