Petr Horáček has posted comments on this change. Change subject: net: add a resolv.conf writer ......................................................................
Patch Set 3: Code-Review-1 (9 comments) https://gerrit.ovirt.org/#/c/62705/3/lib/vdsm/network/ip/resolv.py File lib/vdsm/network/ip/resolv.py: PS3, Line 33: , 'r' 'r' is default, we can drop it. PS3, Line 38: nameservers = [] : for words in _parse_lines(file_text): : if words[0] == 'nameserver': : nameservers.append(words[1]) : return nameservers return [words[1] for words in _parse_lines(file_test) if words[0] == 'nameserver'] Line 51: yield words Line 52: Line 53: Line 54: def update(nameservers): Line 55: lines = [_conf_header()] header_lines Line 56: for ns in nameservers: Line 57: lines.append('nameserver ' + ns) Line 58: Line 59: for words in _parse_lines(_read_resolv_conf()): PS3, Line 56: for ns in nameservers: : lines.append('nameserver ' + ns) ns_lines = ['nameserver ' + ns for ns in nameservers] PS3, Line 59: for words in _parse_lines(_read_resolv_conf()): : if words[0] != 'nameserver': : lines.append(' '.join(words)) other_lines = [' '.join(words) for words in _parse_lines(...) if words[0] != 'nameserver'] Line 59: for words in _parse_lines(_read_resolv_conf()): Line 60: if words[0] != 'nameserver': Line 61: lines.append(' '.join(words)) Line 62: Line 63: with open(DNS_CONF_FILE, 'w') as f: lines = + itertools.chain.from_iterable([header_lines, ns_lines, other_lines]) Line 64: f.writelines(line + '\n' for line in lines) Line 65: Line 66: Line 67: def _conf_header(): Line 63: with open(DNS_CONF_FILE, 'w') as f: Line 64: f.writelines(line + '\n' for line in lines) Line 65: Line 66: Line 67: def _conf_header(): it can be defined as a constant or memoized. https://gerrit.ovirt.org/#/c/62705/3/tests/network/resolv_test.py File tests/network/resolv_test.py: PS3, Line 57: namedTemporaryDir We should use mock.mock_open() in new tests. see ifacquire_test.py. PS3, Line 68: [ : resolv._conf_header() + '\n'] + : ['nameserver {}\n'.format(ns) for ns in nameservers] + : self.RESOLV_CONF[1:3]) can we define it in another constant? -- To view, visit https://gerrit.ovirt.org/62705 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41b81b04fbe3e3e2398f28cb16fb0dbbc0382249 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com> Gerrit-Reviewer: Petr Horáček <phora...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
