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

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

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

libvirtconnection: Replace assert with AssertionError

The code wrongly assumed that assert always exists. When running in
optimized mode, the check would be skipped, and instead of getting an
AssertionError, which is the expected error for programmer error
(starting the eventloop twice), we could get a confusing
RuntimeException or RuntimeError from Thread.start (depending on Python
version).

RuntimeError misused in the standard library for all kinds of errors
that do not have builtin errors. It is particularry bad option when used
for usage error.

Change-Id: Icf1564f81f4c1fbf77ccaff6d93c047a02d946da
Signed-off-by: Nir Soffer 
---
M lib/vdsm/libvirtconnection.py
1 file changed, 2 insertions(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/64/34364/1

diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py
index 5430c82..009f8b7 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)

Francesco Romani has posted comments on this change.

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

Patch Set 1: Code-Review+1

I agree with the rationale behind this patch, but I'm wondering if we should make use of an even more specific exception or not.

AssertionError could be good enough for starter, hence +1

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

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

Patch Set 1:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/13073/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12915/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/12125/ : FAILURE

[Vinzenz Feenstra]

Vinzenz Feenstra has posted comments on this change.

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

Patch Set 1: Code-Review+1

[Maor Lipchuk]

Maor Lipchuk has posted comments on this change.

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

Patch Set 1: Code-Review+1

[Nir Soffer]

Nir Soffer has posted comments on this change.

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

Patch Set 1:

ping?

[Jenkins CI RO]

Jenkins CI RO has abandoned this change.

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

Abandoned

Abandoned due to no activity - please restore if still relevant

gerrit-hooks has posted comments on this change.

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

Patch Set 3:

* Update tracker: IGNORE, no Bug-Url found

[Nir Soffer]

Nir Soffer has restored this change.

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

Restored