New subject: Change in vdsm[master]: assert: Replace assertions with AssertionError

Thursday, 26 June 2014

Nir Soffer has uploaded a new change for review.

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

assert: Replace assetion with AssertionError

If Python is run with -O flag, assertions are removed. This may casue a
useful error message to be hidden, making it harder to debug, or worse,
code invoke inside an assert will not run, changing the flow of the
"optimized" code.

This patch replaces assertions with raising AssertionError, ensuring
that when the impossible happen, we will get an expection.

This patch, together with http://gerrit.ovirt.org/29302 replace all the
asserts in vdsm code. The rest of the assert are in tests where they
safe and useful.

Change-Id: I8c60c65b283f4343448bb9eaf6ccf2526b188db0
Signed-off-by: Nir Soffer <nsoffer(a)redhat.com&gt;
---
M lib/vdsm/libvirtconnection.py
M lib/vdsm/netinfo.py
M vdsm/rpc/BindingXMLRPC.py
M vdsm/rpc/process-schema.py
M vdsm/storage/misc.py
5 files changed, 25 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/29307/1

diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py
index b1332db..260de23 100644
--- a/lib/vdsm/libvirtconnection.py
+++ b/lib/vdsm/libvirtconnection.py
@@ -37,7 +37,8 @@
         self.__thread = None
 
     def start(self):
-        assert not self.run
+        if self.run:
+            raise AssertionError("EventLoop is running")
         self.__thread = threading.Thread(target=self.__run,
                                          name="libvirtEventLoop")
         self.__thread.setDaemon(True)
diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py
index 8d4dc19..78055b4 100644
--- a/lib/vdsm/netinfo.py
+++ b/lib/vdsm/netinfo.py
@@ -986,8 +986,9 @@
     def getBondingForNic(self, nic):
         bondings = list(self.getBondingsForNic(nic))
         if bondings:
-            assert len(bondings) == 1, \
-                "Unexpected configuration: More than one bonding per nic"
+            if len(bondings) != 1:
+                raise AssertionError("Unexpected configuration: More than "
+                                     "one bonding per nic")
             return bondings[0]
         return None
 
@@ -1005,13 +1006,19 @@
 
         for port in ports:
             if port in self.vlans:
-                assert vlan is None
+                if vlan is not None:
+                    raise AssertionError("Unexpected vlan: %s exepected: None"
+                                         % (vlan,))
                 nic = getVlanDevice(port)
                 vlan = getVlanID(port)
-                assert self.vlans[port]['iface'] == nic
+                if self.vlans[port]['iface'] != nic:
+                    raise AssertionError("Unexpected iface: %s expected: %s" %
+                                         (self.vlans[port]['iface'], nic))
                 port = nic
             if port in self.bondings:
-                assert bonding is None
+                if bonding is not None:
+                    raise AssertionError("Unexpected bonding: %s expected: "
+                                         "None" % bonding)
                 bonding = port
                 lnics += self.bondings[bonding]['slaves']
             elif port in self.nics:
diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py
index 42aad1b..ffe233d 100644
--- a/vdsm/rpc/BindingXMLRPC.py
+++ b/vdsm/rpc/BindingXMLRPC.py
@@ -1088,7 +1088,8 @@
                 logLevel = logging.TRACE
             displayArgs = args
             if f.__name__ == 'vmDesktopLogin':
-                assert 'password' not in kwargs
+                if 'password' in kwargs:
+                    raise AssertionError("Unexpected kwarg: password")
                 if len(args) > 3:
                     displayArgs = args[:3] + ('****',) + args[4:]
 
diff --git a/vdsm/rpc/process-schema.py b/vdsm/rpc/process-schema.py
index ee88d93..51ec55c 100755
--- a/vdsm/rpc/process-schema.py
+++ b/vdsm/rpc/process-schema.py
@@ -103,7 +103,9 @@
                    'xxx': []})
 
     # Pop a blank line
-    assert('' == lines.pop(0))
+    line = lines.pop(0)
+    if line != '':
+        raise AssertionError("Expected empty line: %s" % line)
 
     # Grab the entity description.  It might span multiple lines.
     symbol['desc'] = lines.pop(0)
@@ -111,7 +113,9 @@
         symbol['desc'] += lines.pop(0)
 
     # Pop a blank line
-    assert ('' == lines.pop(0))
+    line = lines.pop(0)
+    if line != '':
+        raise AssertionError("Expected empty line: %s" % line)
 
     # Populate the rest of the human-readable data.
     # First try to read the parameters/members information.  We are finished
diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py
index d0869ee..4e7d9ba 100644
--- a/vdsm/storage/misc.py
+++ b/vdsm/storage/misc.py
@@ -697,7 +697,9 @@
 
     def exit(self):
         with self._cond:
-            assert self._busy, "Attempt to exit a barrier without entering"
+            if not self._busy:
+                raise AssertionError("Attempt to exit a barrier without "
+                                     "entering")
             self._busy = False
             self._cond.notifyAll()
 


-- 
To view, visit http://gerrit.ovirt.org/29307
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c60c65b283f4343448bb9eaf6ccf2526b188db0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com&gt;

    

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

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