Ondřej Svoboda has posted comments on this change. Change subject: netinfo: make getNicsVlanAndBondingForNetwork more readable, drop asserts ......................................................................
Patch Set 4: (1 comment) Hoping to make you change your mind :-) https://gerrit.ovirt.org/#/c/45095/4/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 917 Line 918 Line 919 Line 920 Line 921 > I guess your assumptions are correct, and having a test is great, but havin The whole point of rewriting the function was that we needed to remove the asserts (as you pointed out, they may be compiled out so they would be useless if semantics was incorrect) and maybe more importantly, figuring out what the heck the complicated code meant. After I understood (again) I replaced the asserts with (hopefully) better code, a long comment, an explanatory commit message that can be found by git blaming and a test covering the supported configurations. That's much more than what we usually have :-) The asserts' conditions have been tested by years of use in production (let me admit though that this function is not very important – but I envision it is moved to KernelConfig code, where it serves its purpose, sometime). Let me strees that unless the assumed network hierarchy ever changes, this code (as part of the whole VDSM networking subsystem) is correct. If this were systemd or kernel code, I would try much, much harder to make it even more readable and stuff it with assertions. I learned that VDSM, on the other hand, follows Pythonic intuition. Correctness follows from readability. I say, let's stick with this version :-) -- To view, visit https://gerrit.ovirt.org/45095 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If8535dbd29314c97e4519b160a47c9ea6489b5a3 Gerrit-PatchSet: 4 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: Nir Soffer <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
