Edward Haas has posted comments on this change.

Change subject: network: write out nameservers to ifcfg files
......................................................................


Patch Set 11: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/61184/11/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

PS11, Line 520: 
> but it is used by callers. we may end up with TypeError. anyway, it is not 
I am less strict about this, but you can just extract it to a separate patch 
before this one.


https://gerrit.ovirt.org/#/c/61184/11/tests/network/config_network_test.py
File tests/network/config_network_test.py:

Line 158:             legacy_switch.validate_network_setup(networks, {})
Line 159:         self.assertEqual(cneContext.exception.errCode,
Line 160:                          errors.ERR_BAD_PARAMS)
Line 161: 
Line 162:     @MonkeyPatch(dns, 'get_host_nameservers', lambda: 
FAKE_NAMESERVERS)
There is no need for this test here, canonicalize is usually implicitly checked 
most of the times.

We could add a canonicalize test, but that is not in our focus at the moment.
Line 163:     def test_assign_nameservers_to_management_network(self):
Line 164:         networks = {
Line 165:             'management': {'defaultRoute': 'true'},
Line 166:             'vm': {},


-- 
To view, visit https://gerrit.ovirt.org/61184
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie10ee7938b26a7f3b2b7be80bc1a2a83cd1c376c
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Edward Haas <[email protected]>
Gerrit-Reviewer: Fabian Deutsch <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <[email protected]>
Gerrit-Reviewer: Petr Horáček <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to