Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should triopg support manual resource management? #15

Open
shamrin opened this issue Jan 27, 2021 · 6 comments
Open

Should triopg support manual resource management? #15

shamrin opened this issue Jan 27, 2021 · 6 comments

Comments

@shamrin
Copy link
Contributor

shamrin commented Jan 27, 2021

Today I was trying to do something that the documentation says is not supported:

triopg does not support manual resource management

And then I've realized I was the one who wrote the above 🤣 I think I was wrong. We should provide some form of manual resource management. Here is a simple web app using quart-trio. It uses the same pattern that Flask recommends for database connections:

from quart_trio import QuartTrio
from quart import current_app, g

app = QuartTrio(__name__)

async def get_db():
    if 'db' not in g:
        g.db = await current_app.config.db_pool.acquire()
    return g.db

@app.teardown_appcontext
async def teardown_db(exception):
    db = g.pop('db', None)
    if db is not None:
        await db.release()

Handle can then use get_db(). Each handler / task gets a separate database connection:

@app.route('/', methods=['POST'])
async def hello():
    db = await get_db()
    # ...

But, as I said, await pool.acquire() is not supported in triopg 🤷‍♂️:

    g.db = await current_app.config.db_pool.acquire()
TypeError: object TrioPoolAcquireContextProxy can't be used in 'await' expression
@shamrin
Copy link
Contributor Author

shamrin commented Jan 27, 2021

On a related note, quart-trio should probably support per-handler asynccontextmanager-like pattern:

@app.appcontext_some_thing_or_the_other
async def get_db():
    async with current_app.config.db_pool.acquire() as db:
        g.db = db
        yield g.db

Or even pytest fixture-like:

@app.appcontext
async def db():
    async with current_app.config.db_pool.acquire() as conn:
        yield conn

@app.route('/', methods=['POST'])
async def hello(db):  # saying `db` here automatically injects it from db(), in pytest-like fashion
    db = await get_db()
    # ...

But it's offtopic here :)

@shamrin
Copy link
Contributor Author

shamrin commented Jan 27, 2021

(I did submit an issue to Quart: https://gitlab.com/pgjones/quart/-/issues/390)

@touilleMan
Copy link
Member

I strongly think we shouldn't provide this manual resource management given 1) it's not in trio philosophy 2) provide both approach means people would be very tempted to use the manual management approach most of the time (given it's the approach they are most familiar with)

For you particular case, I would say you should create a nursery that would run two coroutines: one for Quart's app.run_task and another one to hand the database connection.
Of course this is tedious, so your issue on Quart side definitely makes sense ;-)

@njsmith
Copy link
Member

njsmith commented Feb 9, 2021

I don't think trio philosophy has a strong opinion on manual resource management? context managers are great when they work, and one of the reasons structured concurrency is nice is that it lets them work in more cases, but I'm pretty sure there will always be cases where resource lifetimes won't match up perfectly with some lexical block.

@nocturn9x
Copy link

Honestly I don't think allowing manual resource management makes sense. At the end of the day, context managers provide a cleaner and more structured way of doing the same thing so I don't see why triopg should break trio's structured concurrency model just to be familiar to asyncio users

@shamrin
Copy link
Contributor Author

shamrin commented Feb 12, 2021

@nocturn9x Manual resource management is not without precedent in Trio. For example, built-in memory channel:

Channel objects can be closed by calling aclose or using async with.

https://trio.readthedocs.io/en/stable/reference-core.html?highlight=aclose#trio.open_memory_channel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants