OD103 cursor-commit¶
Business code commits the cursor, breaking the request's transaction.
| Category | Severity | Series | Needs the index |
|---|---|---|---|
| Correctness | Warning | all | no |
What it reports¶
A call to commit() on the request's cursor: cr.commit(), self.env.cr.commit() or
self._cr.commit(), a cr parameter included.
It stays silent on a commit on a cursor the function opens with a .cursor() call, which is a
transaction of its own: a name it binds to the call, with with registry.cursor() as cr: or
cr = registry.cursor(), whether on self.pool, self.env.registry or db_connect(dbname), or
the cursor of an environment built on such a name or on the call itself, with
api.Environment(cr, ...), self.env(cr=cr) or with_env(). A name bound to anything else, such
as cr = new_cr or self.env.cr, may hold the request's cursor and is reported.
After an if or a loop, a name holds such a cursor only when every path reaching the commit binds
it to one. The paths out of an if are its branches that do not end in return, raise,
continue, break or sys.exit(), and the path past them when it has no else. A branch, or a
loop body, also ends in a block that leaves on every way through it: a with whose own last
statement does, even under a manager such as contextlib.suppress that may swallow the exception,
an if with an else, a match whose last case, such as case _:, matches anything and has no
guard, and a try whose finally leaves, or that has no finally and whose handlers leave along
with its body or its else. Each run of a loop body starts from every path reaching it: the one
before the loop, the end of a run that gets there and each continue. The paths out of the loop are
each break in its body outside a nested loop, wherever it sits, and, unless it is while True:,
the point where a run would start, through the loop's else. So cr = self.env.cr, then
if new: cr = registry.cursor(), then cr.commit() is reported. A path on which the function never
bound the name adds nothing, since Python leaves it unbound there, and neither does one binding it
to None or False, since code tests such a name before using it. The handlers of a try and the
cases of a match are read in the order of the source.
A function nested in it sees the names as they stand on the paths reaching its def, each binding
after the def taking over since the function may run later, unless a parameter of its own hides
them. A lambda sees none of them.
Why it matters¶
Odoo runs each request, cron job and RPC call in one transaction: it commits when the call ends and rolls everything back when it raises. A commit in the middle makes the work done so far permanent, so an error later leaves half of the operation applied and the user is told it failed. Odoo also retries a whole request when PostgreSQL reports a concurrent update, and the retry repeats the committed work, sending the same message or creating the same record twice.
Committing on purpose has its place, such as a cron job that processes a large batch and keeps its
progress when a later record fails; from 19.0, ir.cron's _commit_progress() does that. Review
such a call and suppress the finding on its line.
Example¶
class ProductTemplate(models.Model):
_inherit = "product.template"
def action_push_prices(self):
for template in self:
template._send_price_to_shop()
template.x_price_pushed_on = fields.Datetime.now()
self.env.cr.commit()
If the third template fails, the first two stay marked as pushed while the user sees an error.
class ProductTemplate(models.Model):
_inherit = "product.template"
def action_push_prices(self):
for template in self:
template._send_price_to_shop()
template.x_price_pushed_on = fields.Datetime.now()