ets-labs / python-dependency-injector

Dependency injection framework for Python
https://python-dependency-injector.ets-labs.org/
BSD 3-Clause "New" or "Revised" License
3.7k stars 296 forks source link

FastAPI async resouce not being closed #595

Open John98Zakaria opened 2 years ago

John98Zakaria commented 2 years ago

I was trying out the async resource provider to verify that it is actually closing the resource. However, I noticed that it is not closing the resource.

After deep investigations I found out that the @inject decorator on the API makes python think that the function is not an async generator.

FastAPI decides whether the dependency is a generator on this line https://github.com/tiangolo/fastapi/blob/master/fastapi/dependencies/utils.py#L522

Which then tries to inspect the code using the builtin inspect function

def is_gen_callable(call: Callable[..., Any]) -> bool:
    if inspect.isgeneratorfunction(call):
        return True
    call = getattr(call, "__call__", None)
    return inspect.isgeneratorfunction(call)

def is_async_gen_callable(call: Callable[..., Any]) -> bool:
    if inspect.isasyncgenfunction(call):
        return True
    call = getattr(call, "__call__", None)
    return inspect.isasyncgenfunction(call)

Which is returning false for async generators decorated with @inject

I have attached the test code.

I'll try investigating the @inject decorator To see whether it could be fixed

import contextlib
import os
from functools import partial

from dependency_injector import containers, providers, resources
from dependency_injector.wiring import Provide, inject, Closing
from fastapi import FastAPI, Depends
from pydantic import BaseSettings, Field
from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine, AsyncSession
from sqlalchemy.orm import sessionmaker

os.environ["sql_alchemy_str"] = "sqlite+aiosqlite:///database.db"

class DatabaseSettings(BaseSettings):
    sql_alchemy_str: str = Field(env="sql_alchemy_str")

async def make(engine=None):
    print(f"Make {engine}")
    session = sessionmaker(
        engine, expire_on_commit=False, class_=AsyncSession
    )
    yield session()
    with open("kill.txt", "w") as f:
        f.write("killed")
    print("kill")

class DatabaseContainer(containers.DeclarativeContainer):
    config = providers.Configuration[DatabaseSettings]("DatabaseConfig")
    alchemy_engine: AsyncEngine = providers.Singleton(create_async_engine, config.sql_alchemy_str, echo=True)
    async_session = providers.Resource(make, alchemy_engine)

app = FastAPI()

@app.get("/Hello")
@inject
async def say_hi(session: AsyncSession = Depends(Closing[Provide[DatabaseContainer.async_session]], use_cache=False)):
    print(f"Got {session}")

container = DatabaseContainer()
container.config.from_pydantic(DatabaseSettings())
container.wire(modules=[__name__])

@app.on_event("startup")
async def startup_event():
    print(container.alchemy_engine())
John98Zakaria commented 2 years ago

I suppose we have a regression?

https://github.com/ets-labs/python-dependency-injector/issues/574

Voldemat commented 1 year ago

I encountered the same issue, shutdown method of resource just don`t called after handler scope ends.

Voldemat commented 1 year ago

I think the main problem here that Resource is more about global context, I mean that resource initialized and shutdowns with server. But in our case we need something like AsyncFactory, that will build different objects on every request and shutdown them properly after handler scopes ends. From the docs: "Resource provider is similar to Singleton. Resource initialization happens only once. You can make injections and use provided instance the same way like you do with any other provider."

John98Zakaria commented 1 year ago

I think the main problem here that Resource is more about global context, I mean that resource initialized and shutdowns with server. But in our case we need something like AsyncFactory, that will build different objects on every request and shutdown them properly after handler scopes ends. From the docs: "Resource provider is similar to Singleton. Resource initialization happens only once. You can make injections and use provided instance the same way like you do with any other provider."

I would argue against that. If you put a breakpoint at https://github.com/tiangolo/fastapi/blob/master/fastapi/dependencies/utils.py#L522

You will see how fastapi handles Depends(...) objects.

Fastapi is unable to recognize that the async generator function (the one creating/closing the db) a generator is. I instead it sees it as a normal function which makes it skip the yield call and nothing close the resource. You can verify that by converting everything to a sync function. Then the resource will be closed and everything would be work as expected.

The only once that is mentioned for being created once /request dependency chain.

Voldemat commented 1 year ago

I would argue against that. If you put a breakpoint at https://github.com/tiangolo/fastapi/blob/master/fastapi/dependencies/utils.py#L522

I put breakpoint there and in my case fastapi receive Closing marker, not generator. I tried to use sync generator, but it also did not closed. So I think problem is that Closing marker don`t work with Depends, If you call function manually, resource closes how it should.

Voldemat commented 1 year ago

I would argue against that. If you put a breakpoint at https://github.com/tiangolo/fastapi/blob/master/fastapi/dependencies/utils.py#L522

I use fastapi==0.79.1 and dependency-injector==4.40.0. In your case fastapi also receives Closing marker at that breakpoint.

panicoenlaxbox commented 1 year ago

Hi, I'm facing the same problem here

Closing + Depends does not work

Medhi83 commented 1 year ago

Hi guys! I have the same issue, there is any update on it, or any workaround ?

Example of my init method to provide connection resource :

async def init_connection(db_pool) :
    async with db_pool.connection() as connection:
        yield connection

_Where dbpool is an instance of Database from databases module.

dosuken123 commented 5 months ago

Also, this Depends(Provide[...]) usage will spin up a new thread per request due to solved = await run_in_threadpool(call, **sub_values). This has some major issues:

Hence, I don't recommend following the FastAPI example. https://python-dependency-injector.ets-labs.org/examples/fastapi.html.