Ondřej Svoboda has posted comments on this change.

Change subject: netinfo: rework reporting of DHCPv4/6 on network devices
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/46430/6/lib/vdsm/netinfo.py
File lib/vdsm/netinfo.py:

Line 594:     """
Line 595:     updated_networking = {}
Line 596:     underlying_device = network_info['iface']
Line 597: 
Line 598:     for devices in ('bridges', 'vlans', 'bondings', 'nics'):
> I am inclined to call only NICs "interfaces" and to call bridges/vlans/bond
Renamed to network_device in PS 9.
Line 599:         dev_info = networking[devices].get(underlying_device)
Line 600:         if dev_info:
Line 601:             cfg = {}
Line 602:             updated_networking[devices] = {


Line 611: 
Line 612:     return updated_networking
Line 613: 
Line 614: 
Line 615: def _update_nested_dict(original, replacement):
> Of course I do. But imagine how clumsy a targeted function would be: we are
("Of course I do know what should be updated", not "Of course I need the 
helper")

I kept _update_nested_dict because it really does the job in a clean way, and 
is easy to understand.
Line 616:     """
Line 617:     Use a replacement dict's fields in two ways: if a value is a 
dictionary,
Line 618:     update it recursively. Otherwise replace the value (e.g. a 
string).
Line 619:     """


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaffdc836f8f64ecdc0a7e37ef50c228032f99696
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Ido Barkan <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <[email protected]>
Gerrit-Reviewer: Petr Horáček <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to