[Cubicweb] NativeSQLSource.doexec auto-rollback

aurélien campéas 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
> been
> >     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
> 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?
>

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
> 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
> >       '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.

Regards,
Aurélien.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cubicweb.org/pipermail/cubicweb/attachments/20141230/07f0f8fe/attachment-0050.html>


More information about the Cubicweb mailing list