[Cubicweb] NativeSQLSource.doexec auto-rollback
Christophe de Vienne
christophe at unlish.com
Mon Dec 29 12:40:41 CET 2014
Le 28/12/2014 00:39, aurélien campéas a écrit :
> 2014-12-27 18:36 GMT+01:00 Christophe de Vienne <christophe at unlish.com
> <mailto:christophe at unlish.com>>:
> Digging a little in the source and looking for the 'transaction has been
> rolled back', I found the NativeSQLSource.doexec function, which is
> def doexec(self, cnx, query, args=None, rollback=True):
> cursor = cnx.blablabla
> except Exception as ex:
> self.critical('sql: xxx' % ex.xxx)
> if rollback:
> self.critical('transaction has been rolled back')
> # some clever code to translate the exception in a CW one
> # if possible
> Several things are bothering me:
> - The 'rollback' feature, if anyhow useful (I do not think it is),
> should not have such a default: doexec is, in my understanding, a low
> level function. And rolling back a transaction is a high-level
> decision. But since the doexec function is called by many intermediate
> functions without passing any 'rollback' parameter, we end up with a
> high-level decision made in a low-level function.
> At this point in the flow, the low-level transaction has been rolled back,
> and we cannot defer for too long emitting a rollback event at the cw
> "cnx" level.
What do you mean by "the low-level transaction has been rolled back"?
Do you refer to something lower-level than doexec?
Why would an error rollback the transaction?
> - If an error occur at the sql level, it will be transmitted or
> translated to a CW error, which is fine. But if the exception is
> raised and handled in a higher level function, why do we make it a
> critical error?
> - Admitting that rolling-back automatically on errors is wanted in some
> cases, how can it be considered 'critical'? The caller will receive
> the original exception anyway, it is its job to decide if it is
> critical or not.
> So basically, I would like to propose 2 things:
> - Remove the 'critical' logs, and change them into into 'debug', or
I chose 'debug' and 'info' levels for the rollback and sql error logs.
I wonder if using 'warning' could make sense.
> - Remove the 'rollback' option, or at least change its default value.
> I really thing this feature has nothing to do in this place.
> Should this be moved up to the Connection object ?
It could be done in the connection context manager __exit__, if the
exceptions are not handled and if a 'auto-rollback' option is set when
opening the connection (which could be the default).
It would make more sense to me, and allow a user-provided exception
handler to make different decisions.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 473 bytes
Desc: OpenPGP digital signature
More information about the Cubicweb