fastapi icon indicating copy to clipboard operation
fastapi copied to clipboard

[QUESTION] Depends auto wraps with contextmanager which creates duplicate code

Open kdorsel opened this issue 4 years ago • 4 comments

First check

  • [x] I used the GitHub search to find a similar issue and didn't find it.
  • [x] I searched the FastAPI documentation, with the integrated search.
  • [x] I already searched in Google "How to X in FastAPI" and didn't find any information.

Description

I have a database pool with which I have to define two exact method except for the @asynccontextmanager decorator.

class Pool:
    async def get_con(self):
        con = await self.pool.acquire()
        try:
            yield con
        finally:
            await self.pool.release(con)

    @asynccontextmanager
    async def __call__(self):
        con = await self.pool.acquire()
        try:
            yield con
        finally:
            await self.pool.release(con)

pool = Pool()

One is used in my internal code:

async with pool() as con:
    con.fetch()

And the other with the Depends:

@router.get("/item/{id}")
async def get(id: UUID, db: Connection = Depends(pool.get_con)):

In the documentation there is a line which states

But you don't have to use the decorators for FastAPI dependencies (and you shouldn't).

If you try:

@router.get("/item/{id}")
async def get(id: UUID, db: Connection = Depends(pool)):

I actually get an AttributeError. I'm assuming pool is yieled instead of pool.__call__.

AttributeError: '_AsyncGeneratorContextManager' object has no attribute 'fetchrow'

Which makes it pretty clear that I cannot use a contextmanager in a Depends, but this creates duplicate and non consistent code when getting a db connection.

Is this the way it is? Can Depends work with both regular yields and contextmanagers directly?

kdorsel avatar Apr 27 '20 14:04 kdorsel

That whole bottom section in the documentation is quite confusing, because while FastAPI does use @contextmanager/@asynccontextmanager for dependencies, it is in a pretty complicated way. From the user's (your) point of view, dependencies have nothing to do with context managers.

In the docs

You don't have to use the decorators for FastAPI dependencies (and you shouldn't).

should really read:

You can't use the decorators for FastAPI dependencies.

and in fact I would argue that whole section is unnecessary and only leads to confusion.

In your specific case, to eliminate the duplicate code you can do:

class Pool:
    async def get_con(self):
        con = await self.pool.acquire()
        try:
            yield con
        finally:
            await self.pool.release(con)

    __call__ = asynccontextmanager(get_con)

then continue to use Depends(pool.get_con).

A separate issue is that the pattern of

def foo():
    with bar() as b:
        yield b

@app.get("/")
async def endpoint(f: foo = Depends()):
    ...

means you're opening context managers and wrapping them up in the context manager under the hood of the dependency system.

retnikt avatar Apr 29 '20 14:04 retnikt

I would agree with you that the documentation is confusing, but this clears things up with a nice simple solution, thanks!

kdorsel avatar Apr 29 '20 18:04 kdorsel

This helped me out, thanks all. Here's what mine looks like:

async def database_session():
    """
    Returns a database Session for use with fastapi Depends()
    """
    session = async_scoped_session(_db_session, scopefunc=current_task)
    try:
        yield session
    finally:
        await session.remove()
@app.on_event("startup")
async def create_admin_user():
    """
    Makes sure the app's admin user exists on startup
    """
    DatabaseSession = asynccontextmanager(database_session)
    async with DatabaseSession() as session:
        do_db_stuff_with_db_session(session)
@router.get("/")
async def get_user(user_id: str, session: AsyncSession = Depends(database_session)):
    do_stuff_with_db_session(session)

evindunn avatar Aug 07 '22 20:08 evindunn

I don't know why this is not allowed... Maybe I'm missing something? But I guess either a PR allowing that or a PR improving the docs should be created 🤔

Kludex avatar Dec 29 '22 11:12 Kludex