tortoise / aerich

A database migrations tool for TortoiseORM, ready to production.
https://github.com/tortoise/aerich
Apache License 2.0
823 stars 93 forks source link

Close Tortoise connections properly #121

Closed AulonSal closed 3 years ago

AulonSal commented 3 years ago

Addresses issue #120

Chooses the simplest solution of always running Tortoise.close_connections() in the coro wrapper.

long2ice commented 3 years ago

Hi, this update I tried before but had some problems, you can try some commands in different databases in local

AulonSal commented 3 years ago

Ignore this, I figured it out

Tried it with postgresql and there indeed is a problem, what the update is doing is basically what tortoise.run_async does but it's only meant for simple scripts so I went and made an asynccontextmanager-decorator and wrapped the database-touching functions in it and that seems to work(tested with sqlite and postgresql) but I still want to figure out if it can work at a higher level and why it failed in the coro wrapper.

I made TortoiseCloseConnections a class based decorator/context-manager but if it's never going to be used in a with clause it can be replaced with the following decorator:

def tortoise_connection_cleanup(func):
    @wraps(func)
    async def inner(*args, **kwds):
        try:
            return await func(*args, **kwds)
        finally:
            await Tortoise.close_connections()
    return inner
AulonSal commented 3 years ago

Everything should work now. I could alternatively import run_async from tortoise and define wrapper as:

    def wrapper(*args, **kwargs):
        if f.__name__ != 'cli':
            run_async(f(*args, **kwargs)
        else:
            loop = asyncio.get_event_loop()
            loop.run_until_complete(f(*args, **kwargs))

        return wrapper

if you'd prefer that.

long2ice commented 3 years ago

OK, thanks fot that! Just one thing, could you update version and changelog? Which in pyproject.toml and aerich/__init__.py.

long2ice commented 3 years ago

Thanks!