[node-patches] Change in ovirt-node[master]: Allow nested transactions

fabiand at fedoraproject.org fabiand at fedoraproject.org
Mon Oct 7 13:33:21 UTC 2013


Fabian Deutsch has posted comments on this change.

Change subject: Allow nested transactions
......................................................................


Patch Set 4:

(1 comment)

Yes - wrt your last comment, Ryan.

I don't think we wan't more. I've added one comment for the implementation and please add your testcase into the doctest part of the Transaction class.

....................................................
File src/ovirt/node/utils/__init__.py
Line 211:                                                     "Element") % element)
Line 212:             else:
Line 213:                 self._prepared_elements.append(element)
Line 214:                 element.prepare()
Line 215:         return True
Can't we just do it like this:

            if not issubclass(element.__class__, Transaction.Element) and not issubclass(element.__class__, Transaction):
                raise exceptions.PreconditionError(("%s is no Transaction." +
                                                    "Element") % element)
            self._prepared_elements.append(element)
            element.prepare()
        return True

because if element is type-of Transaction, the it also implements .prepare() (which will be called in line 214)
Line 216: 
Line 217:     def commit(self):
Line 218:         for element in self:
Line 219:             self.logger.debug("Committing element '%s'" % element)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia74e1dff22827f0b5fb47ba57eaa72c304998474
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>
Gerrit-Reviewer: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes



More information about the node-patches mailing list