Change in vdsm[master]: assert: Replace assetion with AssertionError

asegurap at redhat.com asegurap at redhat.com
Fri Jun 27 11:29:56 UTC 2014


Antoni Segura Puimedon has posted comments on this change.

Change subject: assert: Replace assetion with AssertionError
......................................................................


Patch Set 1: Code-Review-1

(6 comments)

Thanks for the change. Some string suggestions.

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 <nsoffer at redhat.com>
Line 5: CommitDate: 2014-06-26 22:12:24 +0300
Line 6: 
Line 7: assert: Replace assetion with AssertionError
s/assetion/assertion/
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/
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 network "%s" that already has a vlan "%s" port.' % (port, network, vlan)
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"' % (self.vlans[port]['iface'], nic)
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 network "%s" that already has a bond "%s" port.' % (port, network, bonding)
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'
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 <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list