Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Dan Kenigsberg has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Looks good to me, approved heh, I'm not crazy for the interim variable either. No biggy. -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Dan Kenigsberg has submitted this change and it was merged. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. refactoring: getMaxMtu updated implementation using max builtin. Improvement to getMaxMtu implementation which now is more succint by using max builtin function. Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Signed-off-by: Giuseppe Vallarelli --- M vdsm/netconf/ifcfg.py 1 file changed, 2 insertions(+), 5 deletions(-) Approvals: Martin Sivák: Looks good to me, but someone else must approve Giuseppe Vallarelli: Verified Antoni Segura Puimedon: Looks good to me, but someone else must approve Dan Kenigsberg: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Antoni Segura Puimedon has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Looks good to me, but someone else must approve I'll leave the docstring fixing as a thing for the future. -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Antoni Segura Puimedon has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: (2 inline comments) File vdsm/netconf/ifcfg.py Line 682: f.writelines(entries) Line 683: f.close() Line 684: Line 685: def getMaxMtu(self, nics, mtu): Line 686: """ Would you mind changing the docstring, so that it states that it gets the maximum mtu from parameter/current netinfo mtu for the nic? Line 687: Get the max MTU value from configuration/parameter Line 688: Line 689: :param nics: list of nics Line 690: :type nics: list Line 696: it check if a vlan, bond that have a higher mtu value Line 697: """ Line 698: Line 699: nics_mtu = [int(netinfo.getMtu(nic)) for nic in nics] Line 700: return max(mtu, *nics_mtu) :-) I preferred it the other way just with one argument on each line... But I will not complain to much about the extra variable. Line 701: Line 702: def setNewMtu(self, network, bridged, _netinfo=None): Line 703: """ Line 704: Set new MTU value to network and its interfaces -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Martin Sivák has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Looks good to me, but someone else must approve Looks good to me now. -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2280/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1441/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2344/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Giuseppe Vallarelli has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Verified -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Giuseppe Vallarelli has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: (1 inline comment) File vdsm/netconf/ifcfg.py Line 695: getMaxMtu return the highest value in a connection tree, Line 696: it check if a vlan, bond that have a higher mtu value Line 697: """ Line 698: Line 699: return max(mtu, *[int(netinfo.getMtu(nic)) for nic in nics]) Done Line 700: Line 701: def setNewMtu(self, network, bridged, _netinfo=None): Line 702: """ Line 703: Set new MTU value to network and its interfaces -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2280/ (3/3) -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1441/ (1/3) -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 2: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2344/ (2/3) -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Martin Sivák has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: Looks good to me, but someone else must approve (1 inline comment) This looks good, except for one style comment. File vdsm/netconf/ifcfg.py Line 695: getMaxMtu return the highest value in a connection tree, Line 696: it check if a vlan, bond that have a higher mtu value Line 697: """ Line 698: Line 699: return max(mtu, *[int(netinfo.getMtu(nic)) for nic in nics]) Can you please split this into two lines? Save the list of mtus and then call max() on it. Although this is correct, it is not easy to read. Line 700: Line 701: def setNewMtu(self, network, bridged, _netinfo=None): Line 702: """ Line 703: Set new MTU value to network and its interfaces -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Livnat Peer Gerrit-Reviewer: Martin Sivák Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Giuseppe Vallarelli has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: Verified -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Giuseppe Vallarelli Gerrit-Reviewer: Vinzenz Feenstra Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: Build Successful http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2278/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1439/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2342/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
Giuseppe Vallarelli has uploaded a new change for review. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. refactoring: getMaxMtu updated implementation using max builtin. Improvement to getMaxMtu implementation which now is more succint by using max builtin function. Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Signed-off-by: Giuseppe Vallarelli --- M vdsm/netconf/ifcfg.py 1 file changed, 1 insertion(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/14743/1 diff --git a/vdsm/netconf/ifcfg.py b/vdsm/netconf/ifcfg.py index 58dc501..efa83e0 100644 --- a/vdsm/netconf/ifcfg.py +++ b/vdsm/netconf/ifcfg.py @@ -695,12 +695,8 @@ getMaxMtu return the highest value in a connection tree, it check if a vlan, bond that have a higher mtu value """ -for nic in nics: -mtuval = int(netinfo.getMtu(nic)) -if mtuval > mtu: -mtu = mtuval -return mtu +return max(mtu, *[int(netinfo.getMtu(nic)) for nic in nics]) def setNewMtu(self, network, bridged, _netinfo=None): """ -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/2342/ (3/3) -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/1439/ (2/3) -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: refactoring: getMaxMtu updated implementation using max buil...
oVirt Jenkins CI Server has posted comments on this change. Change subject: refactoring: getMaxMtu updated implementation using max builtin. .. Patch Set 1: Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/2278/ (1/3) -- To view, visit http://gerrit.ovirt.org/14743 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e3beef38675b2f74f2945247b8af1de3eeebc90 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Giuseppe Vallarelli Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
