Issue: commit_on_success raises an OperationalError after db.begin() since 2.4.5
Author: felixonmars
State: closed
Created: 2015-10-11 14:42:05

The following code works with peewee 2.4.4 but fails with 2.4.5+. Replacing the commit_on_success part with a simple `db.commit()` won't fail, though. Any hints for the behavior change?

```
from peewee import SqliteDatabase

db = SqliteDatabase(":memory:")
db.set_autocommit(False)
db.connect()
db.begin()
db.set_autocommit(True)

@db.commit_on_success
def i_want_to_commit():
    return

i_want_to_commit()
```

The error:

```
Traceback (most recent call last):
  File "test.py", line 13, in <module>
    i_want_to_commit()
  File "/usr/lib/python2.7/site-packages/peewee.py", line 2880, in inner
    with self.transaction():
  File "/usr/lib/python2.7/site-packages/peewee.py", line 3330, in __enter__
    self._begin()
  File "/usr/lib/python2.7/site-packages/peewee.py", line 3314, in _begin
    self.db.begin()
  File "/usr/lib/python2.7/site-packages/peewee.py", line 2985, in begin
    self.execute_sql('BEGIN %s' % lock_type, require_commit=False)
  File "/usr/lib/python2.7/site-packages/peewee.py", line 2833, in execute_sql
    self.commit()
  File "/usr/lib/python2.7/site-packages/peewee.py", line 2688, in __exit__
    reraise(new_type, new_type(*exc_value.args), traceback)
  File "/usr/lib/python2.7/site-packages/peewee.py", line 2825, in execute_sql
    cursor.execute(sql, params or ())
peewee.OperationalError: cannot start a transaction within a transaction
```


Your code is very strange. Why are you beginning a transaction explicitly? I don't think this is a bug.


This snippet is simplified from deepin-movie's source code, and as I am only porting the software to Arch, I am not very sure the original purpose of delayCommit either.

Link to original code that having the same problem: https://github.com/linuxdeepin/deepin-movie/blob/release/2.3/src/models/playlist.py


I have no idea but I think that these bugs are not bugs in peewee, but someone doing something weird.

Setting autocommit to False and calling `begin()` on a SQLite database will execute a `BEGIN` statement. When you open a transaction, via the decorator, `begin()` is again called.

Peewee maintains transactional state but not at the `begin()` API level. Instead, you should use the `transaction()` or `atomic()` methods.

```
db = SqliteDatabase(":memory:")
db.connect()
with db.atomic():
    # Do some stuff.
    i_want_to_commit()

@db.atomic()
def i_want_to_commit():
    return
```


Sorry, I just want to clarify my comment about transaction state.

Your best bet with Peewee is to use the `db.atomic()` method. This method works as both a context-manager or a decorator, so you can:

``` python

with db.atomic():
    with db.atomic():
        something_else()


@db.atomic()
def something_else()
    pass
```

The nice thing about `atomic()` is that it's smart and knows when to use transactions or savepoints.

By explicitly calling `begin()`, the code you referenced is bypassing Peewee's transactional state tracker, so the call to `@commit_on_success` fails to see that a transaction was opened higher up the call-stack, and throws an error because it tries to open one.

Does that help?


Thank you. I believe the original code was meant to delay commit for 500ms, and currently I don't see a way without calling `begin()` explicitly. Does `db.atomic()` work the same way as `@commit_on_success` and would work even if `begin()` was called explicitly?


> I believe the original code was meant to delay commit for 500ms, and currently I don't see a way without calling begin() explicitly.

Well, I'm curious about that. Why is it necessary to delay for 500ms?

> Does db.atomic() work the same way as @commit_on_success and would work even if begin() was called explicitly?

No, and the commit_on_success should really be considered deprecated. You can use `db.atomic()` or `db.transaction()`. Atomic has the benefit that nested blocks will use savepoints. If you just use transactions, you will only get a single transaction on the outermost block.


I see. I'll try to reach the original author and figure out the reason for 500ms delay. Many thanks for the info!