[Cubicweb] NativeSQLSource.doexec auto-rollback
aurelien.campeas at gmail.com
Tue Dec 30 18:24:14 CET 2014
2014-12-29 12:40 GMT+01:00 Christophe de Vienne <christophe at unlish.com>:
> 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
> > rolled back', I found the NativeSQLSource.doexec function, which is
> > basically:
> > def doexec(self, cnx, query, args=None, rollback=True):
> > cursor = cnx.blablabla
> > try:
> > cursor.execute()
> > except Exception as ex:
> > self.critical('sql: xxx' % ex.xxx)
> > if rollback:
> > cnx.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
> > level function. And rolling back a transaction is a high-level
> > decision. But since the doexec function is called by many
> > functions without passing any 'rollback' parameter, we end up with
> > high-level decision made in a low-level function.
> > At this point in the flow, the low-level transaction has been rolled
> > 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?
After reception of an IntegrityError, you can be sure that the sql backend
wiped the current transaction clean.
> > - 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
> > 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
> > 'info'.
> > +1
> Ticket+patch: http://www.cubicweb.org/ticket/4801120
> I chose 'debug' and 'info' levels for the rollback and sql error logs.
> I wonder if using 'warning' could make sense.
Choosing warning would keep the information more visible.
Integrity errors are rare events in general and we may want
to keep them easily spottable in a log stream.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Cubicweb