Change in vdsm[master]: Add callback to the plethora of retry halting possibilities

List overview All Threads
Download

newer

older

Change in vdsm[master]: netConf:...

Change in vdsm[master]: spec: fix...

smizrahi＠redhat.com

20 Sep 2012 20 Sep '12

8:03 a.m.

Saggi Mizrahi has uploaded a new change for review.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Add callback to the plethora of retry halting possibilities

Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M tests/miscTests.py M vdsm/storage/misc.py 2 files changed, 35 insertions(+), 1 deletion(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/8093/1

diff --git a/tests/miscTests.py b/tests/miscTests.py index bec6539..c8bcf09 100644 --- a/tests/miscTests.py +++ b/tests/miscTests.py @@ -621,6 +621,35 @@ self.assertEquals(misc._alignData(1, 1), (1, 1, 1))

+class RetryTests(TestCaseBase): + def testStopCallback(self): + counter = [0] + limit = 4 + def stopCallback(): + counter[0] += 1 + if counter[0] == limit: + return True + + return False + + def foo(): + print counter[0] + if counter[0] == (limit - 1): + return + + raise RuntimeError("If at first you don't succeed, try, try again." + "Then quit. There's no point in being a damn" + "fool about it.") + # W. C. Fields + + self.assertRaises(RuntimeError, misc.retry, foo, tries=(limit - 1), sleep=0, + stopCallback=stopCallback) + + counter[0] = 0 + misc.retry(foo, RuntimeError, tries=limit, sleep=0, + stopCallback=stopCallback) + + class ValidateDDBytes(TestCaseBase): def testValidInputTrue(self): """ diff --git a/vdsm/storage/misc.py b/vdsm/storage/misc.py index 161726b..ef0c74e 100644 --- a/vdsm/storage/misc.py +++ b/vdsm/storage/misc.py @@ -728,7 +728,7 @@

def retry(func, expectedException=Exception, tries=None, - timeout=None, sleep=1): + timeout=None, sleep=1, stopCallback=None): """ Retry a function. Wraps the retry logic so you don't have to implement it each time you need it. @@ -741,6 +741,8 @@ the method. It will just not run it if it ended after the timeout. :param sleep: Time to sleep between calls in seconds. + :param stopCallback: A function that takes no parameters and invokes a + bail-out when it returns with a positive value. """ if tries in [0, None]: tries = -1 @@ -761,6 +763,9 @@ if (timeout > 0) and ((time.time() - startTime) > timeout): raise

+ if stopCallback is not None and stopCallback(): + raise + time.sleep(sleep)

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

Gerrit-MessageType: newchange Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com

Show replies by date

abaron＠redhat.com

5 Nov 5 Nov

2:03 p.m.

Ayal Baron has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

.................................................... File tests/miscTests.py Line 641: "Then quit. There's no point in being a damn" Line 642: "fool about it.") Line 643: # W. C. Fields Line 644: Line 645: self.assertRaises(RuntimeError, misc.retry, foo, tries=(limit - 1), sleep=0, this raises because you've hit the max number of retries, not because stopCallback returned true. Line 646: stopCallback=stopCallback) Line 647: Line 648: counter[0] = 0 Line 649: misc.retry(foo, RuntimeError, tries=limit, sleep=0,

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

Gerrit-MessageType: comment Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com

abaron＠redhat.com

2:05 p.m.

Ayal Baron has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 1: (1 inline comment)

.................................................... File tests/miscTests.py Line 633: return False Line 634: Line 635: def foo(): Line 636: print counter[0] Line 637: if counter[0] == (limit - 1): also, the test is needlessly confusing. foo should not touch counter, only stopCallback should. foo should always fail to keep it simple. Line 638: return Line 639: Line 640: raise RuntimeError("If at first you don't succeed, try, try again." Line 641: "Then quit. There's no point in being a damn"

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

zhshzhou＠linux.vnet.ibm.com

7:14 p.m.

Zhou Zheng Sheng has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 2: I would prefer that you didn't submit this

(2 inline comments)

.................................................... File tests/miscTests.py Line 694: Line 695: self.assertRaises(RuntimeError, misc.retry, foo, tries=limit, Line 696: sleep=0, stopCallback=stopCallback) Line 697: # Make sure we had the proper amount of iterations before failing Line 698: self.assertEquals(counter[0], (limit - 1)) It seems that this test for testing "retry" hits the "limit" without calling "stopCallback".

To test an early halt from "stopCallback", "tries" shout be larger than "limit", and at last "aseertEquals(counter[0], limit)".

Maybe there can be two tests, one for hitting the "limit" before "stopCallback", one for early halt from "stopCallback". Line 699: Line 700: Line 701: class ValidateDDBytes(TestCaseBase): Line 702: def testValidInputTrue(self):

.................................................... File vdsm/storage/misc.py Line 743: the method. It will just not run it if it ended after the Line 744: timeout. Line 745: :param sleep: Time to sleep between calls in seconds. Line 746: :param stopCallback: A function that takes no parameters and invokes a Line 747: bail-out when it returns with a positive value. The explanation for stopCallback is very cryptic to me non-English speaker...

I have to read the code to get the meaning. Line 748: """ Line 749: if tries in [0, None]: Line 750: tries = -1 Line 751:

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

Gerrit-MessageType: comment Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com

shaohef＠linux.vnet.ibm.com

6 Nov 6 Nov

1:19 a.m.

ShaoHe Feng has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 2: (2 inline comments)

.................................................... File tests/miscTests.py Line 675: Line 676: Line 677: class RetryTests(TestCaseBase): Line 678: def testStopCallback(self): Line 679: counter = [0] why using list, not int "counter = 0"? Line 680: limit = 4 Line 681: Line 682: def stopCallback(): Line 683: counter[0] += 1

.................................................... File vdsm/storage/misc.py Line 745: :param sleep: Time to sleep between calls in seconds. Line 746: :param stopCallback: A function that takes no parameters and invokes a Line 747: bail-out when it returns with a positive value. Line 748: """ Line 749: if tries in [0, None]: how about? if not tries: tries = -1 Line 750: tries = -1 Line 751: Line 752: if timeout in [0, None]: Line 753: timeout = -1

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

zhshzhou＠linux.vnet.ibm.com

1:27 a.m.

Zhou Zheng Sheng has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 2: (1 inline comment)

.................................................... File tests/miscTests.py Line 675: Line 676: Line 677: class RetryTests(TestCaseBase): Line 678: def testStopCallback(self): Line 679: counter = [0] stopCallback can not assign to the variable counter directly, it can only read it. So a list here is a trick to simulate pointer in C or reference in CPP. Line 680: limit = 4 Line 681: Line 682: def stopCallback(): Line 683: counter[0] += 1

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

zhshzhou＠linux.vnet.ibm.com

9:48 p.m.

Zhou Zheng Sheng has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com

shaohef＠linux.vnet.ibm.com

8 Nov 8 Nov

7:33 a.m.

ShaoHe Feng has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4: I would prefer that you didn't submit this

(1 inline comment)

.................................................... File tests/Makefile.am Line 45: restTests.py \ Line 46: restData.py \ Line 47: storageMailboxTests.py \ Line 48: tcTests.py \ Line 49: utilsTests.py \ should sort this list? Line 50: vdsClientTests.py \ Line 51: remoteFileHandlerTests.py \ Line 52: resourceManagerTests.py \ Line 53: $(NULL)

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

smizrahi＠redhat.com

3 Dec 3 Dec

10:41 a.m.

Saggi Mizrahi has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4: (1 inline comment)

.................................................... File tests/Makefile.am Line 45: restTests.py \ Line 46: restData.py \ Line 47: storageMailboxTests.py \ Line 48: tcTests.py \ Line 49: utilsTests.py \ It's not sorted anyway, anther patch Line 50: vdsClientTests.py \ Line 51: remoteFileHandlerTests.py \ Line 52: resourceManagerTests.py \ Line 53: $(NULL)

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

abaron＠redhat.com

7 Dec 7 Dec

3:47 p.m.

Ayal Baron has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4: Looks good to me, approved

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

shaohef＠linux.vnet.ibm.com

9 Dec 9 Dec

3:51 a.m.

ShaoHe Feng has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4: Looks good to me, but someone else must approve

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

smizrahi＠redhat.com

2 Jan 2 Jan

1:19 p.m.

Saggi Mizrahi has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4: Verified

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

Gerrit-MessageType: comment Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com

Dan Kenigsberg

7:40 p.m.

Dan Kenigsberg has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 4:

שנה טובה

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

Gerrit-MessageType: comment Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com

Jenkins CI RO

7:44 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/551/ (2/2)

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

Gerrit-MessageType: comment Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

Jenkins CI RO

7:44 p.m.

oVirt Jenkins CI Server has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 5:

Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/516/ (1/2)

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

Dan Kenigsberg

7:45 p.m.

Dan Kenigsberg has posted comments on this change.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Patch Set 5: Verified; Looks good to me, approved

no idea why JGit had an issue with this rebase. copying scores.

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

Dan Kenigsberg

7:45 p.m.

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Add callback to the plethora of retry halting possibilities ......................................................................

Add callback to the plethora of retry halting possibilities

Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Signed-off-by: Saggi Mizrahi smizrahi@redhat.com --- M tests/Makefile.am A tests/utilsTests.py M vdsm/utils.py 3 files changed, 53 insertions(+), 1 deletion(-)

Approvals: Dan Kenigsberg: Verified; Looks good to me, approved

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

Gerrit-MessageType: merged Gerrit-Change-Id: Idb5a2158f008b41133352dcfb4926ad21dcceea1 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server

4196

Age (days ago)

4301

Last active (days ago)

vdsm-patches@lists.fedorahosted.org

16 comments

6 participants

tags (0)

participants (6)

abaron＠redhat.com
Dan Kenigsberg
Jenkins CI RO
shaohef＠linux.vnet.ibm.com
smizrahi＠redhat.com
zhshzhou＠linux.vnet.ibm.com