Ido Barkan has posted comments on this change. Change subject: net: MTU should be an integer all over VDSM. ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/50657/1//COMMIT_MSG Commit Message: Line 6: Line 7: net: MTU should be an integer all over VDSM. Line 8: Line 9: It should be serialized as such and be converted to a string only Line 10: when written to files, and immedietly upon the start of setupNetworks > immediately Done Line 11: API call. Line 12: Line 13: Change-Id: Id7ba110d85b8ebd459c29ea438a68b8e55ccb1da https://gerrit.ovirt.org/#/c/50657/1/lib/vdsm/network/api.py File lib/vdsm/network/api.py: Line 975: Line 976: if 'mtu' not in attrs: Line 977: attrs['mtu'] = DEFAULT_MTU Line 978: else: Line 979: attrs['mtu'] = int(attrs['mtu']) > this function should add missing defaults, not convert passed attributes this conversion does not justify a function call. moreover, it emphasizes that the _missing_ default is, in fact, an int. Just like DEFAULT_MTU is. This makes the to if/else cases symmetric. Line 980: Line 981: Line 982: def setSafeNetworkConfig(): Line 983: """Declare current network configuration as 'safe'""" -- To view, visit https://gerrit.ovirt.org/50657 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7ba110d85b8ebd459c29ea438a68b8e55ccb1da Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Edward Haas <edwa...@redhat.com> Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki <mmire...@redhat.com> 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/mailman/listinfo/vdsm-patches