Zhou Zheng Sheng has uploaded a new change for review.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
tests: add a rollback manager for easy undoing
Sometimes we need to perform a series of operations: op[0], op[1], ... op[N] These operations may allocate files, locks, connections, and op[K] may depend on op[K-1] 's result Consider these are contexts, after constructing contexts, we want to perform some computing using these contexts. Exception may be raised in the complicated construction stage of the contexts or when we're using the contexes.
So if op[K] fails, we need to: undo op[K-1], undo op[K-2], ... undo op[0] These undo operations release the resources, or if all the operations succeed, at last we need to: undo op[N], undo op[N-1], ... undo op[0]
Furthermore, We want to suppress the exceptions occured in "undo op[X]", and continue the rollback, so that all the resources can be freed.
At last, we want to see the first exception raised so that we can fix the root cause, so we want to have the oldest exception occured in this batch of operation reraised.
This patch proposes a concise framework to do this kind of rollback. It's an upgrade version of contextlib.contextmanager .
Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Signed-off-by: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com --- M tests/Makefile.am A tests/rollbackManagerTests.py M tests/testrunner.py 3 files changed, 161 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/8671/1
diff --git a/tests/Makefile.am b/tests/Makefile.am index 2b61dde..02f48ab 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -45,6 +45,7 @@ persistentDictTests.py \ restTests.py \ restData.py \ + rollbackManagerTests.py \ tcTests.py \ vdsClientTests.py \ remoteFileHandlerTests.py \ diff --git a/tests/rollbackManagerTests.py b/tests/rollbackManagerTests.py new file mode 100644 index 0000000..db3a5a0 --- /dev/null +++ b/tests/rollbackManagerTests.py @@ -0,0 +1,85 @@ +# +# Copyright IBM Corp. 2012 +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# +import glob +import os +import tempfile +import uuid + +from testrunner import VdsmTestCase as TestCaseBase +from testrunner import rollbackManager + + +class ContextError(Exception): + pass + + +class ConsumerError(Exception): + pass + + +class TestRollbackManager(TestCaseBase): + def setUp(self): + self.tmpdirPrefix = 'testrollback' + str(uuid.uuid4()) + + @rollbackManager + def tempfiles(self, fileCount, excClass, rollback): + dirPath = tempfile.mkdtemp(prefix=self.tmpdirPrefix) + undo = lambda: os.rmdir(dirPath) + rollback.append(undo) + + for i in range(0, fileCount): + path = os.path.join(dirPath, str(i)) + with open(path, "wb") as f: + undo = \ + lambda path=path: os.remove(path) + rollback.append(undo) + f.write(str(i)) + + if excClass is not None: + raise excClass("context error") + + return dirPath + + def testExceptionInContext(self): + def exceptionInContext(): + with self.tempfiles(10, ContextError): + pass + + self.assertRaises(ContextError, exceptionInContext) + # Directory and files should be removed + self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) + + def testExceptionInConsumer(self): + def exceptionInConsumer(): + with self.tempfiles(10, None): + raise ConsumerError("consumer error") + + self.assertRaises(ConsumerError, exceptionInConsumer) + # Directory and files should be removed + self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) + + def testNormalConsumer(self): + fileCount = 10 + with self.tempfiles(fileCount, None) as dirPath: + for i in range(0, fileCount): + with open(os.path.join(dirPath, str(i)), "rb") as f: + self.assertEquals(int(f.read()), i) + # Directory and files should be removed + self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) diff --git a/tests/testrunner.py b/tests/testrunner.py index cdbc9d3..f9cc323 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -22,6 +22,7 @@ import os import unittest from functools import wraps +from contextlib import contextmanager
from nose import config from nose import core @@ -239,6 +240,80 @@ return False
+def rollbackManager(transaction): + ''' + A contextmanager-like manager for easy undoing. It's an upgraded + contextlib.contextmanager and can manage a variable number of contextes. + + It is used as a decorator to a function. There must exist a parameter + named "rollback" of the decorated function. Then the function can treat + the "rollback" as a list and append undo operations(lambdas, closures, ...) + to the list. The function will be put in the "with" statement, the return + value of the function will be assigned to the "as" variable. If an + exception is raised, whether it's raised in the function or in the block + under the "with" statement, the registered undo operations will be played + in reverse order. When peforming rollback, exceptions will be swalloweded + to let rollback continue, at last, the earliest exception with original + line number and stack trace infomation will be raised. + + Simple example: + _________________________________________ + @rollbackManager + def foo(a, b, rollback): + x = allocate_X(a) + rollback.append(lambda: release_X(x)) + + # Need not to catch the exception when allocating Y + # and release x in a "try..final" block. + # The rollback Manager will do that. + y = allocate_Y_using(x) + rollback.append(lambda: release_Y(y)) + + # Need not to catch the exception when computing z + # and release x, y. Let rollback Manager do it for you. + z = do_something_with(x, y) + return z + + with foo(blah1, blah2) as z: + visit(z) + _________________________________________ + When the "with" block is exited, resource y will be released first, then + resource x. If exception is raised when constructing y, then only the + allocated x will be released, and the original exception will be re-raised. + ''' + + @contextmanager + def wrapper(*args, **kwargs): + rollback = [] + exception = None + traceback = None + try: + yield transaction(rollback=rollback, *args, **kwargs) + except Exception as e: + # keep the original exception and traceback info + exception = e + traceback = sys.exc_info()[2] + finally: + rollback.reverse() + _playRollback(rollback, exception, traceback) + return wrapper + + +def _playRollback(rollback, exception=None, traceback=None): + for undo in rollback: + try: + undo() + except Exception as e: + # keep the earliest exception info + if not exception: + exception = e + # keep the original traceback info + traceback = sys.exc_info()[2] + # re-raise the earliest exception + if exception: + raise exception, None, traceback + + if __name__ == '__main__': if "--help" in sys.argv: print("testrunner options:\n"
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 1:
I split the rollback manager part in this patch from a bigger patch at gerrit.ovirt.org/#/c/8182/
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 2: Verified
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3: Verified
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Vinzenz Feenstra has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(1 inline comment)
The only thing I would like to see is the tests list in Makefile.am to be sorted, other than that it looks good to me. So basically a minority for the change, however it'd be useful Thank you.
.................................................... File tests/Makefile.am Line 43: permutationTests.py \ Line 44: persistentDictTests.py \ Line 45: restTests.py \ Line 46: restData.py \ Line 47: rollbackManagerTests.py \ Can we please sort this list by name? Thank you Line 48: tcTests.py \ Line 49: vdsClientTests.py \ Line 50: remoteFileHandlerTests.py \ Line 51: resourceManagerTests.py
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Saggi Mizrahi has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3: I would prefer that you didn't submit this
I think you just want to use storage.misc.DeferableContext()
Although in my opinion if a resource doesn't have it's own context you should just create one for it.
Makes for a simpler interface.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3:
Thanks Vinzenz and Saggi.
Creating context manager for each resource is good. When the number of the resource is variable or large, for example, creating multiple volumes according to a configuration file, we have to use contextlib.nested(). However according to Python official document, this function is being deprecated, and there are some limitations.
storage.misc.DeferableContext() is good, and it re-raises the last exception, while rollbackManager re-raises the first exception with exc_info. When investigating the problem, the first exception is more probably to be the root cause, and exc_info is very useful.
There will be little different comparing DeferableContext() with rollbackManager. I can try to write examples.
DeferableContext Example
buildContext1(arg, df): r1 = step1(arg) df.prependDefer(lambda: undo1(r1)) r2 = step2(r1) df.prependDefer(lambda: undo2(r2)) return r2
buildContext2(arg, df): for i in arg: r = step(i) df.prependDefer(lambda: undo(r)) return r
with DeferableContext() as df: r1 = buildContext1(arg1, df) r2 = buildContext2(arg2, df) consume(r1, r2)
--------
rollbackManager Example
@rollbackManager buildContext1(arg, rollback): r1 = step1(arg) rollback.append(lambda: undo1(r1)) r2 = step2(r1) rollback.append(lambda: undo2(r2)) return r2
@rollbackManager buildContext2(arg, rollback): for i in arg: r = step(i) rollback.append(lambda: undo(r)) return r
with buildContext1(arg1) as r1, buildContext2(arg2) as r2: consume(r1, r2)
Which style do you like? If you think it's better to re-use DeferableContext, may I propose a way to keep and re-raise the first exception in DeferableContext with exc_info?
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(3 inline comments)
DeferableContext can support the rollback as rollbackManager, but it will raise the lastException when it exit,that's different. form the the Simple example foo of this patch, it give us the advance of first Exception.
actually, I prefer rollbackManager. it can also support positive sequence, not only fallback.
the problem is that Makes for a simpler interface.
you have to sorted makefile list again, though you have sorted it.
.................................................... File tests/Makefile.am Line 43: permutationTests.py \ Line 44: persistentDictTests.py \ Line 45: restTests.py \ Line 46: restData.py \ Line 47: rollbackManagerTests.py \ the original list is not sorted. hehe Line 48: tcTests.py \ Line 49: vdsClientTests.py \ Line 50: remoteFileHandlerTests.py \ Line 51: resourceManagerTests.py
.................................................... File tests/testrunner.py Line 251: to the list. The function will be put in the "with" statement, the return Line 252: value of the function will be assigned to the "as" variable. If an Line 253: exception is raised, whether it's raised in the function or in the block Line 254: under the "with" statement, the registered undo operations will be played Line 255: in reverse order. When peforming rollback, exceptions will be swalloweded swalloweded -> swallowed ? Line 256: to let rollback continue, at last, the earliest exception with original Line 257: line number and stack trace infomation will be raised. Line 258: Line 259: Simple example:
Line 303: for undo in rollback: Line 304: try: Line 305: undo() Line 306: except Exception as e: Line 307: # keep the earliest exception info yes, the earliest exception info is more useful than the latest exception info. maybe the latest exception info was cause by the earliest exception. and you example give the reason. Line 308: if not exception: Line 309: exception = e Line 310: # keep the original traceback info Line 311: traceback = sys.exc_info()[2]
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3:
zheng sheng zhou:
you can make it support positive sequence, not only rollback.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Vinzenz Feenstra has posted comments on this change.
Change subject: tests: add a rollback manager for easy undoing ......................................................................
Patch Set 3: (2 inline comments)
.................................................... File tests/Makefile.am Line 43: permutationTests.py \ Line 44: persistentDictTests.py \ Line 45: restTests.py \ Line 46: restData.py \ Line 47: rollbackManagerTests.py \ I know, but nevertheless it would be a good idea to start with it. Line 48: tcTests.py \ Line 49: vdsClientTests.py \ Line 50: remoteFileHandlerTests.py \ Line 51: resourceManagerTests.py
.................................................... File tests/testrunner.py Line 251: to the list. The function will be put in the "with" statement, the return Line 252: value of the function will be assigned to the "as" variable. If an Line 253: exception is raised, whether it's raised in the function or in the block Line 254: under the "with" statement, the registered undo operations will be played Line 255: in reverse order. When peforming rollback, exceptions will be swalloweded yepp should be swallowed Line 256: to let rollback continue, at last, the earliest exception with original Line 257: line number and stack trace infomation will be raised. Line 258: Line 259: Simple example:
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 4: Verified
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 4: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File tests/rollbackContextTests.py Line 62: self.tempfiles(10, UndoError, rollback) Line 63: Line 64: self.assertRaises(UndoError, exceptionInUndo) Line 65: # Directory and files should be removed Line 66: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) why need a wildcard? glob.glob(self.tmpdirPrefix + "*") can not list the subdir or subfile.
if a file named self.tmpdirPrefix + 'another', this test can be failed. surely, it is impossible for uuid.uuid4. Line 67: Line 68: def testExceptionUnderContext(self): Line 69: def exceptionUnderContext(): Line 70: with RollbackContext() as rollback:
Line 82: for i in range(0, fileCount): Line 83: with open(os.path.join(dirPath, str(i)), "rb") as f: Line 84: self.assertEquals(int(f.read()), i) Line 85: # Directory and files should be removed Line 86: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) I have check the RollbackContext code. you implement a mechanism that catch the first Exception.
can you construct more testes that the Exception was raised during the RollbackContext?
example: So during the "with RollbackContext() as rollback", you can make a new file. not add it's remove operation to undo. then os.rmdir(dirPath) will raise an Exception. with RollbackContext() as rollback: dirPath = self.tempfiles(fileCount, None, rollback) open(os.path.join(dirPath, "Newfile"), "rb")
Open a file, but not close it. then both os.remove(path) and os.rmdir(dirPath) will raise an Exception. with RollbackContext() as rollback: dirPath = self.tempfiles(fileCount, None, rollback) open(os.path.join(dirPath, str(5)), "rb")
just a suggestion.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
ShaoHe Feng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 4: (1 inline comment)
.................................................... File tests/rollbackContextTests.py Line 62: self.tempfiles(10, UndoError, rollback) Line 63: Line 64: self.assertRaises(UndoError, exceptionInUndo) Line 65: # Directory and files should be removed Line 66: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) dirPath is better? Line 67: Line 68: def testExceptionUnderContext(self): Line 69: def exceptionUnderContext(): Line 70: with RollbackContext() as rollback:
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 4: (2 inline comments)
Thanks ShaoHe. I will update the patch according to your suggestions.
.................................................... File tests/rollbackContextTests.py Line 62: self.tempfiles(10, UndoError, rollback) Line 63: Line 64: self.assertRaises(UndoError, exceptionInUndo) Line 65: # Directory and files should be removed Line 66: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) It does not list the subdir because the subdir is deleted in the undo actions. You have to run a debugger and let it pause before it gets out of the "with" statement to see that dir listed.
I use glob to check if the dir is deleted because in previous patch set, I raise an exception before tempfiles() returns, so the caller does not know the exact path of the temp dir. Now I raise exception in undo actions, so we can get the path always, I can check its existence directly now. Line 67: Line 68: def testExceptionUnderContext(self): Line 69: def exceptionUnderContext(): Line 70: with RollbackContext() as rollback:
Line 82: for i in range(0, fileCount): Line 83: with open(os.path.join(dirPath, str(i)), "rb") as f: Line 84: self.assertEquals(int(f.read()), i) Line 85: # Directory and files should be removed Line 86: self.assertEquals(glob.glob(self.tmpdirPrefix + "*"), []) It seems like you want some tests to verify the exception we caught is the FIRST one. I can pass an index to the exception __init__() methoed, then check if the index is the first one passed.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Zhou Zheng Sheng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 5: Verified
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Vinzenz Feenstra has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 5: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
Dan Kenigsberg has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 5: I would prefer that you didn't submit this
(1 inline comment)
-1 set for visibility.
.................................................... File vdsm/storage/misc.py Line 732: class RollbackContext(DeferableContext): Line 733: ''' Line 734: A context manager for recording and playing rollback. Line 735: ''' Line 736: add = DeferableContext.prependDefer do we really need this alias? For me it is a distraction only. Line 737: Line 738: def __exit__(self, exc_type, exc_value, traceback): Line 739: firstException = exc_value Line 740:
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 6:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/32/
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 6: Verified
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/32/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 5: (1 inline comment)
.................................................... File vdsm/storage/misc.py Line 732: class RollbackContext(DeferableContext): Line 733: ''' Line 734: A context manager for recording and playing rollback. Line 735: ''' Line 736: add = DeferableContext.prependDefer Done Line 737: Line 738: def __exit__(self, exc_type, exc_value, traceback): Line 739: firstException = exc_value Line 740:
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 6: Verified
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Vinzenz Feenstra has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 6: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
ShaoHe Feng has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 6: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Saggi Mizrahi has posted comments on this change.
Change subject: storage.misc: subclass DeferableContext for easy undoing ......................................................................
Patch Set 6: I would prefer that you didn't submit this
Looks like the only difference between the two class is whether to use the first or last exception. I personally don't care.
Also I like the name of you class better
Just change DeferableContext() to whatever you need with the name you want.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/52/ (2/2)
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 7:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/29/ (1/2)
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 7: I would prefer that you didn't submit this
Build Unstable
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/29/ : UNSTABLE
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/52/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 7: Verified
Thanks Vinzenz, Saggi, Dan, ShaoHe. I update the patch according to your suggestions.
Test the patch myself with unit tests and my functional test patches.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Saggi Mizrahi has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 7: Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Saggi Mizrahi has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 7:
I put my +1 so danken knows I approve. You still need to please Jenkins before that gets pushed in.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/58/ (2/2)
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 8:
Build Started http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/35/ (1/2)
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 8:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_manual_gerrit/58/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/35/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Zhou Zheng Sheng has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 8: Verified; Looks good to me, but someone else must approve
Thanks Saggi, I update the patch to pass Jenkins PEP 8 check.
It's just a PEP 8 update for the pieces in the code I touched, so copy Saggi's +1 from last patch. I test the patch in the same way in the last patch.
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
Patch Set 8: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: DeferableContext: Change to RollbackContext for easy undoing ......................................................................
DeferableContext: Change to RollbackContext for easy undoing
We often need to perform a series of operations: op[0], op[1], ... op[N] These operations may allocate files, locks, connections, and op[K] may depend on op[K-1] 's result
Sometimes it's not feasible to create context manager for each resource, because the number of the resource involved in a transaction can be a variable, for example, reading from a configuration file, but there is no way to use a variable number of the "with" statement, and contextlib.nested is being deprecated. So we need a concise framework to do rollback.
This patch subclass DeferableContext and proposes an idiom to do this kind of rollback. DeferableContext re-raises the last exception, while this patch re-raises the first exception, for the earliest exception may be the root cause and most helpful when investigate the problem.
Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Signed-off-by: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com --- M tests/miscTests.py M vdsm/storage/misc.py M vdsm/storage/resourceManager.py 3 files changed, 51 insertions(+), 37 deletions(-)
Approvals: Dan Kenigsberg: Looks good to me, approved Zhou Zheng Sheng: Verified; Looks good to me, but someone else must approve
-- To view, visit http://gerrit.ovirt.org/8671 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: Ibc932637dd81c3becf92de34ea647c1cea136111 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Ryan Harper ryanh@us.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: ShaoHe Feng shaohef@linux.vnet.ibm.com Gerrit-Reviewer: Vinzenz Feenstra vfeenstr@redhat.com Gerrit-Reviewer: Zhou Zheng Sheng zhshzhou@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org