Nir Soffer has posted comments on this change. Change subject: assert: Replace assertions with AssertionError ......................................................................
Patch Set 1: (6 comments) http://gerrit.ovirt.org/#/c/29307/1//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-06-26 20:57:04 +0300 Line 4: Commit: Nir Soffer <[email protected]> Line 5: CommitDate: 2014-06-26 22:12:24 +0300 Line 6: Line 7: assert: Replace assetion with AssertionError > s/assetion/assertion/ Done Line 8: Line 9: If Python is run with -O flag, assertions are removed. This may casue a Line 10: useful error message to be hidden, making it harder to debug, or worse, Line 11: code invoke inside an assert will not run, changing the flow of the Line 7: assert: Replace assetion with AssertionError Line 8: Line 9: If Python is run with -O flag, assertions are removed. This may casue a Line 10: useful error message to be hidden, making it harder to debug, or worse, Line 11: code invoke inside an assert will not run, changing the flow of the > s/invoke/invoked/ Done Line 12: "optimized" code. Line 13: Line 14: This patch replaces assertions with raising AssertionError, ensuring Line 15: that when the impossible happen, we will get an expection. http://gerrit.ovirt.org/#/c/29307/1/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 1007: for port in ports: Line 1008: if port in self.vlans: Line 1009: if vlan is not None: Line 1010: raise AssertionError("Unexpected vlan: %s exepected: None" Line 1011: % (vlan,)) > the message should be: 'Unsupported: Found extra vlan device "%s" in networ Done Line 1012: nic = getVlanDevice(port) Line 1013: vlan = getVlanID(port) Line 1014: if self.vlans[port]['iface'] != nic: Line 1015: raise AssertionError("Unexpected iface: %s expected: %s" % Line 1012: nic = getVlanDevice(port) Line 1013: vlan = getVlanID(port) Line 1014: if self.vlans[port]['iface'] != nic: Line 1015: raise AssertionError("Unexpected iface: %s expected: %s" % Line 1016: (self.vlans[port]['iface'], nic)) > the message should be: 'Unexpected vlan link device "%s", expected "%s"' % Done Line 1017: port = nic Line 1018: if port in self.bondings: Line 1019: if bonding is not None: Line 1020: raise AssertionError("Unexpected bonding: %s expected: " Line 1017: port = nic Line 1018: if port in self.bondings: Line 1019: if bonding is not None: Line 1020: raise AssertionError("Unexpected bonding: %s expected: " Line 1021: "None" % bonding) > the message should be: 'Unsupported: Found extra bond device "%s" in networ Done Line 1022: bonding = port Line 1023: lnics += self.bondings[bonding]['slaves'] Line 1024: elif port in self.nics: Line 1025: lnics.append(port) http://gerrit.ovirt.org/#/c/29307/1/vdsm/rpc/BindingXMLRPC.py File vdsm/rpc/BindingXMLRPC.py: Line 1088: logLevel = logging.TRACE Line 1089: displayArgs = args Line 1090: if f.__name__ == 'vmDesktopLogin': Line 1091: if 'password' in kwargs: Line 1092: raise AssertionError("Unexpected kwarg: password") > I'd say it should read: 'illegal parameter password for vmDesktopLogin' Done Line 1093: if len(args) > 3: Line 1094: displayArgs = args[:3] + ('****',) + args[4:] Line 1095: Line 1096: # Logging current call -- To view, visit http://gerrit.ovirt.org/29307 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c60c65b283f4343448bb9eaf6ccf2526b188db0 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
