aiopeewee icon indicating copy to clipboard operation
aiopeewee copied to clipboard

Refactor connection/atomic/transaction context handling

Open kszucs opened this issue 7 years ago • 7 comments

Due to the use of connection pool by default, the async with self.pool.acquire() idiom is required in most of the database methods. The async with hell gets complicated in the case of atomic operations. The with db.atomic() idiom requires the connection to be shipped downward resulting a deeply nested async context manager, which should look like a single one (from the API perspective). The original peewee implementation uses deque and heap structures to handle the nesting within a single thread.

The most painful parts are here:

  • https://github.com/kszucs/aiopeewee/blob/master/aiopeewee/database.py#L19 (i want to get rid of this abstraction
  • https://github.com/kszucs/aiopeewee/blob/master/aiopeewee/context.py#L72 (currently this is a nightmare)
  • https://github.com/kszucs/aiopeewee/blob/master/aiopeewee/database.py#L189 (mixture of coroutine, context manager, repeated us of async with self.get_conn())

I could use a more asyncio experienced point of view here :) Could someone give me a hand / guidance?

cc @ajdavis @jettify

kszucs avatar Jul 10 '17 07:07 kszucs

I have never used peewee so not sure how I can help, as for transaction I find SQLA explicit session is superior, may be you can introduce, similar session executor for aiopeewee

Session = sessionmaker(bind=engine, autocommit=True)
session = Session()
session.begin()
try:
    item1 = session.query(Item).get(1)
    item2 = session.query(Item).get(2)
    item1.foo = 'bar'
    item2.bar = 'foo'
    session.commit()
except:
    session.rollback()
    raise

jettify avatar Jul 10 '17 20:07 jettify

from aiopeewee import AioModel, AioMySQLDatabase File "/usr/local/lib/python3.5/site-packages/aiopeewee/query.py", line 141 yield row ^ SyntaxError: 'yield' inside async function

leigeng2014 avatar Aug 24 '17 04:08 leigeng2014

Hey @leigeng2014!

Currently aiopeewee requires python 3.6 due to asynchronous generators, can you update your interpreter?

kszucs avatar Aug 25 '17 08:08 kszucs

Hey @kszucs

@app.listener('before_server_start')
async def setup(app, loop):
    # create connection pool
    await db.connect(loop)
    # create table if not exists
    await db.create_tables([User, Blog], safe=True)

The above code for create tables give the following error

File "/home/devpt/virt/pay/lib/python3.6/site-packages/aiopeewee/mysql.py", line 23, in get_tables
    async with self.get_conn() as conn:
  File "/home/devpt/virt/pay/lib/python3.6/site-packages/aiopeewee/database.py", line 145, in get_conn
    raise OperationalError('Database pool has not been initialized')
peewee.OperationalError: Database pool has not been initialized

This is because self.closed is set to True when create_tables is called

kakarot9 avatar Jan 18 '18 12:01 kakarot9

@kakarot9 Do You have pytest-asyncio installed?

kszucs avatar Jan 18 '18 12:01 kszucs

@kszucs yes its installed

kakarot9 avatar Jan 18 '18 12:01 kakarot9

Ohh sorry, wrong scope - I was thinking for a test suite problem. Actually the README example is wrong, connect doesn't accept a loop argument: https://github.com/kszucs/aiopeewee/blob/master/aiopeewee/database.py#L162

Could You check (e.g. print) db.closed and db.pool variables before and after db.connect()?

kszucs avatar Jan 18 '18 12:01 kszucs