On 03/13/2014 08:05 PM, Valentin LAB wrote:
Here's a MP to propose a simple way to add transaction where it could be
needed without breaking legacy code:
https://code.launchpad.net/~0k.io/openobject-server/trunk-new-prevent-cr-commit-to-allow-super-transaction/+merge/210884
If this is okay, it's trivial to encompass openerp's initialize / update
sequence in one go.
Any comments ?
I find this change quite dangerous. Any magic that affects all running
transactions is particularly likely to introduce subtle bugs that we will only
notice long after merging it. The rowcount one is just one example of
not-so-obvious regressions that will happen.
As you've seen, calling legacy code is simply not safe in the context of
savepoints (even after your patch), so we should just be careful with the use
of cr.savepoint() and avoid extra magic.
The point of the cr.savepoint() contextmanager is to automatically close your
savepoint, not to be a safeguard for legacy code.
In addition, I think the API should preserve programmers from surprises as much
as possible. Anyone using commit() explicitly is either:
1. A clueless programmer who should not be using them, and will manage to
screw it, regardless of the API we choose ;
2. Someone who actually knows what SAVEPOINT, COMMIT, ROLLBACK and ROLLBACK
TO SAVEPOINT mean, and will expect cr.commit() and cr.rollback() to do what
they say at all times.
Regarding the issue of commits in auto_init(), I had tried in the past to get
rid of them (they have no reason to be there indeed), but never merged that
patch because it was causing obscure errors that I had no time to investigate.
I would be glad if anyone had the time to look at it.
_______________________________________________
Mailing list: https://launchpad.net/~openerp-expert-framework
Post to : [email protected]
Unsubscribe : https://launchpad.net/~openerp-expert-framework
More help : https://help.launchpad.net/ListHelp
