Issue: Calling db.rollback() instead of tx.rollback() 
Author: stenci
State: closed
Created: 2016-10-07 21:46:26

I found a few errors in my app caused by the incorrect use of `rollback()`: Sometimes I called `db.rollback()` while inside (a function called from) a `with transaction as tx:` block instead of calling `tx.rollback()`. 
1. The `db.rollback()` from inside a transaction works, but does not automatically [begin a new transaction](http://docs.peewee-orm.com/en/latest/peewee/api.html?highlight=rollback%28%29#transaction.rollback)
2. The following `tx.rollback()` fails silently

The little change shown below in `Database.rollback()` and `transaction.rollback()` raises an exception and prevents from making something that stupid.

I'm not even trying to make a pull request because I don't know what type of exception should be risen, how to make a test and, most imporatantly, whether this makes sense or not.

``` python
  class Database(object):
      def rollback(self):
+          if self.transaction_depth():
+              raise Exception('Database.rollback() called while in a transaction (call transaction.rollback() instead).')
          self.get_conn().rollback()


  class transaction(_callable_context_manager):
      def rollback(self, begin=True):
-          self.db.rollback()
+          self.db.get_conn().rollback()
          if begin:
              self._begin()
```

The following code should print only 4 and 8 or should fail. Instead it dangerously prints 4, 7 and 8.

``` python
import peewee

db = peewee.SqliteDatabase(':memory:')

class Table1(peewee.Model):
    n = peewee.IntegerField()
    class Meta:
        database = db

with db.transaction():
    Table1.create_table(True)

with db.transaction() as tx:
    Table1.create(n=1)
    tx.rollback()
    Table1.create(n=2)
    tx.rollback()
    Table1.create(n=3)
    tx.rollback()
    Table1.create(n=4)

with db.transaction() as tx:
    Table1.create(n=5)
    tx.rollback()
    Table1.create(n=6)
    db.rollback()
    Table1.create(n=7)
    tx.rollback()
    Table1.create(n=8)

for rec in Table1.select():
    print(rec.n)
```


```
+          if self.transaction_depth():
+              raise Exception('Database.rollback() called while in a transaction (call transaction.rollback() instead).')
          self.get_conn().rollback()
```

I see the logic in what you're trying to do here, unfortunately I think this one boils down to user error as the docs are quite clear on showing the correct calling conventions.


I agree, Peewee is well written and documented, and the right place to teach how to use it is the documentation, not raising exceptions.

When I write code that I only will re-read, I never write documentation or comments, and if it's small no tests either. Instead I fill it up with `assert`s that will warn me when I will use it in the wrong way.

I opened this issue just to give you some (hopefully constructive) feedback about something that for me wasn't intuitive and led me to a few bugs.


I am playing with `SqliteQueueDatabase`, which doesn't play nice with transactions and which doesn't clearly say it in the documentation. 

This is another case where I would use assert/exceptions to tell the developer that they are using transactions in the wrong way.

Would that improve Peewee? No

Would that help some distracted developer? Yes


For one, there is a big fat warning on the SqliteQueueDatabase stating:

> This is a new module and should be considered alpha-quality software 

Furthermore, I haven't even finished documenting the module. Or stabilized the API. 


> The db.rollback() from inside a transaction works, but does not automatically begin a new transaction

I think this is as it should be. There is a separate method named `begin()`, so it seems quite confusing that the `Database.rollback` would also magically call `Database.begin()`.

This changes, however, when you move to using a transaction as a context manager or function decorator. In these cases, you're saying "I want a transaction for the duration of the wrapped block" -- if you happen to explicitly call commit or rollback on the transaction object, a new transaction begins to maintain the agreement about keeping the wrapped block atomic.