Change in vdsm[master]: tests: add a rollback manager for easy undoing

zhshzhou at linux.vnet.ibm.com zhshzhou at linux.vnet.ibm.com
Thu Nov 8 10:02:52 UTC 2012


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 at linux.vnet.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Royce Lv <lvroyce at linux.vnet.ibm.com>
Gerrit-Reviewer: Ryan Harper <ryanh at us.ibm.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list