Yeela Kaplan has uploaded a new change for review.
Change subject: utils: allow retry of multiple functions ......................................................................
utils: allow retry of multiple functions
Change-Id: I6c805ef33b710fbbe674ac62ae96c3fdf47423e1 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M lib/vdsm/libvirtconnection.py M lib/vdsm/utils.py M tests/functional/vmRecoveryTests.py M tests/miscTests.py M tests/remoteFileHandlerTests.py M tests/utilsTests.py M vdsm/clientIF.py M vdsm/storage/hsm.py M vdsm/supervdsm.py 9 files changed, 18 insertions(+), 13 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/36336/1
diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py index 492a02e..55af991 100644 --- a/lib/vdsm/libvirtconnection.py +++ b/lib/vdsm/libvirtconnection.py @@ -99,7 +99,7 @@
libvirtOpen = functools.partial( libvirt.openAuth, uri, auth, 0) - return utils.retry(libvirtOpen, timeout=10, sleep=0.2) + return utils.retry((libvirtOpen,), timeout=10, sleep=0.2)
def get(target=None, killOnFailure=True): diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index db38e0c..ed4dec6 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -903,13 +903,13 @@ return unicode(self.cmd)
-def retry(func, expectedException=Exception, tries=None, +def retry(funcs, expectedException=Exception, tries=None, timeout=None, sleep=1, stopCallback=None): """ - Retry a function. Wraps the retry logic so you don't have to + Retry functions. Wraps the retry logic so you don't have to implement it each time you need it.
- :param func: The callable to run. + :param funcs: a tuple of the callables to run. :param expectedException: The exception you expect to receive when the function fails. :param tries: The number of times to try. None\0,-1 means infinite. @@ -932,7 +932,9 @@ while True: tries -= 1 try: - return func() + for func in funcs: + res = func() + return res except expectedException: if tries == 0: raise diff --git a/tests/functional/vmRecoveryTests.py b/tests/functional/vmRecoveryTests.py index 37a96f3..0028f62 100644 --- a/tests/functional/vmRecoveryTests.py +++ b/tests/functional/vmRecoveryTests.py @@ -96,7 +96,8 @@
def ensure_vdsm_started(self): vdsm.utils.retry( - self.setUp, expectedException=(socket.error, KeyError), tries=10) + (self.setUp,), expectedException=(socket.error, KeyError), + tries=10)
@ValidateRunningAsRoot def test_vm_recovery(self): diff --git a/tests/miscTests.py b/tests/miscTests.py index a955979..eb8b11d 100644 --- a/tests/miscTests.py +++ b/tests/miscTests.py @@ -1047,7 +1047,7 @@ nice = utils.pidStat(proc.pid).nice self.assertEquals(nice, 10)
- utils.retry(AssertionError, test, tries=10, sleep=0.1) + utils.retry((test,), AssertionError, tries=10, sleep=0.1) proc.kill() proc.wait()
diff --git a/tests/remoteFileHandlerTests.py b/tests/remoteFileHandlerTests.py index 0c62f60..c093c2d 100644 --- a/tests/remoteFileHandlerTests.py +++ b/tests/remoteFileHandlerTests.py @@ -70,7 +70,7 @@ p.stop() test = lambda: self.assertFalse(os.path.exists(procPath))
- utils.retry(test, AssertionError, timeout=4, sleep=0.1) + utils.retry((test,), AssertionError, timeout=4, sleep=0.1)
class RemoteFileHandlerTruncateTests(TestCaseBase): diff --git a/tests/utilsTests.py b/tests/utilsTests.py index cb3da85..75119e8 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -57,7 +57,8 @@ "fool about it.") # W. C. Fields
- self.assertRaises(RuntimeError, utils.retry, foo, tries=(limit + 10), + self.assertRaises(RuntimeError, utils.retry, (foo,), + tries=(limit + 10), sleep=0, stopCallback=stopCallback) # Make sure we had the proper amount of iterations before failing self.assertEquals(counter[0], limit) @@ -111,7 +112,7 @@ try: test = lambda: self.assertEquals(utils.getCmdArgs(sproc.pid), tuple()) - utils.retry(AssertionError, test, tries=10, sleep=0.1) + utils.retry((test,), AssertionError, tries=10, sleep=0.1) finally: sproc.wait()
diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py index 971d5ff..673975a 100644 --- a/vdsm/clientIF.py +++ b/vdsm/clientIF.py @@ -393,7 +393,7 @@ # Trying to run recover process until it works. During that time vdsm # stays in recovery mode (_recover=True), means all api requests # returns with "vdsm is in initializing process" message. - utils.retry(self._recoverExistingVms, sleep=5) + utils.retry((self._recoverExistingVms,), sleep=5)
def _recoverExistingVms(self): try: diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index c8aaf93..fa6eb5a 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3155,7 +3155,7 @@ supervdsm.getProxy().appropriateDevice(guid, thiefId) supervdsm.getProxy().udevTrigger(guid) devPath = os.path.join(devicemapper.DMPATH_PREFIX, guid) - utils.retry(partial(fileUtils.validateQemuReadable, devPath), + utils.retry((partial(fileUtils.validateQemuReadable, devPath),), expectedException=OSError, timeout=QEMU_READABLE_TIMEOUT)
diff --git a/vdsm/supervdsm.py b/vdsm/supervdsm.py index 8e42af2..c29a1f2 100644 --- a/vdsm/supervdsm.py +++ b/vdsm/supervdsm.py @@ -75,7 +75,8 @@ self._manager.register('open') self._log.debug("Trying to connect to Super Vdsm") try: - utils.retry(self._manager.connect, Exception, timeout=60, tries=3) + utils.retry((self._manager.connect,), Exception, + timeout=60, tries=3) except Exception as ex: msg = "Connect to supervdsm service failed: %s" % ex utils.panic(msg)
Dan Kenigsberg has posted comments on this change.
Change subject: utils: allow retry of multiple functions ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/36336/1/lib/vdsm/utils.py File lib/vdsm/utils.py:
Line 933: tries -= 1 Line 934: try: Line 935: for func in funcs: Line 936: res = func() Line 937: return res this makes very little sense to me - you ignore the returned value of all but the last function. If an exception is raised, you cannot tell when it was.
A user can easily write a wrapper function that calls the funcs serialy, and pass it to retry.
And most users need to pass a single function anyway. Line 938: except expectedException: Line 939: if tries == 0: Line 940: raise Line 941:
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: allow retry of multiple functions ......................................................................
Patch Set 1:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_infra_functional_tests_gerrit/28/ : There was an infra issue, please contact infra@ovirt.org
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13655/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14612/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14444/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: utils: allow retry of multiple functions ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_infra_functional_tests_gerrit/29/ : There was an infra issue, please contact infra@ovirt.org
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13722/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14679/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14511/ : SUCCESS
Jenkins CI RO has posted comments on this change.
Change subject: utils: allow retry of multiple functions ......................................................................
Patch Set 2:
Abandoned due to no activity - please restore if still relevant
Jenkins CI RO has abandoned this change.
Change subject: utils: allow retry of multiple functions ......................................................................
Abandoned
Abandoned due to no activity - please restore if still relevant
gerrit-hooks has posted comments on this change.
Change subject: utils: allow retry of multiple functions ......................................................................
Patch Set 2:
* Update tracker: IGNORE, no Bug-Url found
vdsm-patches@lists.fedorahosted.org
-
automation＠ovirt.org -
Dan Kenigsberg -
Jenkins CI RO -
ykaplan＠redhat.com