Yedidyah Bar David has posted comments on this change.

Change subject: packaging: setup: Clear rollback message
......................................................................


Patch Set 1:

Alon Bar-Lev wrote:
> This is implementation specific.
> You assume the ordering is based on order of plugins, while
> it can be random and change at any time.

Indeed.
Specifically:
* Plugins call 
self.environment[otopicons.CoreEnv.MAIN_TRANSACTION].append(plugin-specific-transaction-class)
* self.environment[otopicons.CoreEnv.MAIN_TRANSACTION] is (through otopi's 
transaction plugin) an instance of otopi's transaction.Transaction(). Let's 
call it _main.
* _main.append() appends an element to _elements, which is a list (so keeps its 
order).
* At STAGE_TRANSACTION_BEGIN, _main.prepare() is called, which (also) copies 
_elements to _prepared, also a list
* At STAGE_TRANSACTION_END, we try to commmit, and call _main.abort() on 
exception
* _main.abort loops over _prepared, and calls each element's .abort

(it took me some time to understand all of this, so I document it here for 
whoever that wishes to follow)

I can see several different ways to continue:
1. Merely document - add to Transaction.append()'s docstring that it's 
guaranteed to call relevant methods (prepare, commit, abort, etc.) in the order 
it (append) was called
2. Add some mechanism to otopi.transaction to allow more control over the order 
of how things are called
3. Forget about all this, and just add some logging/warning/etc to 
otopi/src/plugins/otopi/core/transaction.py/_main_end right before 
self._mainTransaction.abort()
4. Forget about all this and do nothing :-), just decide that when we abort 
it's not very important when exactly we say so to the user and with what exact 
words.

Obviously, 3. is the simplest. I did not find it very clean, and also not sure 
if we want, and how, to add ACTION to the message - perhaps by adding ACTION to 
otopi's environment.

I have no problem with 1.

I am obviously against 4.

I currently see no specific reason for doing 2. - adding complexity for no 
significant reason.

What do you think?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieec07031079e23b23f59e166d647988743dffb7b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yedidyah Bar David <[email protected]>
Gerrit-Reviewer: Alex Lourie <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Moran Goldboim <[email protected]>
Gerrit-Reviewer: Ofer Schreiber <[email protected]>
Gerrit-Reviewer: Sandro Bonazzola <[email protected]>
Gerrit-Reviewer: Yedidyah Bar David <[email protected]>
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to