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

fix: Allow to inject async functions into functions wrapped with asynccontextmanager #125

Merged
merged 2 commits into from
Aug 24, 2024

Conversation

Zhenay
Copy link
Contributor

@Zhenay Zhenay commented Aug 24, 2024

When you try to inject synchronous or asynchronous dependencies into an asynchronous generator wrapped with contextlib.asynccontextmanager like this:

async def some_dep() -> int:
    return 5

@asynccontextmanager
@inject
async def some_async_gen(value: Annotated[int, Depends(some_dep)]):
    yield value

you see the error AttributeError: 'solve_async_gen' object has no attribute '_iter'.

The reverse order of the decorators works for sync dependincies only and raises the error AssertionError: You cannot use async dependency `some_dep` at sync main for async ones.

This pull request allows the inject function be used in such context with the direct order of the decorators. I don't see any possibility to provide the ability to use the reverse order because of the way asynccontextmanager is implemented in the standard library:

# contextlib.py - python 3.12
def asynccontextmanager(func):
    @wraps(func)
    def helper(*args, **kwds):
        return _AsyncGeneratorContextManager(func, args, kwds)
    return helper

As you can see, the decorator returns the synchronous wrapper, so when fast_depends execute build_call_model it determines the function as synchronous.


@pytest.mark.anyio
async def test_asynccontextmanager():
async def dep(a: str = "a"):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove default value here and in sync test to make the test much representable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@Lancetnik
Copy link
Owner

Thank you for the PRs a lot! Btw, what are u doing with FastDepends to find such bugs? 😅

@Lancetnik Lancetnik merged commit 492e6b4 into Lancetnik:main Aug 24, 2024
13 checks passed
@Zhenay
Copy link
Contributor Author

Zhenay commented Aug 24, 2024

I do nothing 😅
A collegue of mine created a very minimal DI system for one of our internal projects, but when I showed him FastDepends, he decided that it covers all his needs and he can replace self-written DI with FastDepends.
This internal projects uses async generators very widely, so the issue with injecting async depenedencies into async generators was discovered very quickly.
As far as I know, he have not used asynccontextmanagers with async dependencies yet, but I checked this scenario too as one of the most popular uses of async generators and discovered this issue

@Zhenay
Copy link
Contributor Author

Zhenay commented Aug 24, 2024

And thank you for your awesome projects. I use both FastDepends and FastStream and they are gorgeous

@Tishka17
Copy link

@Zhenay I can suggest trying https://github.com/reagento/dishka/
It has a bit more verbose approach but more features related only to DI. There is an integration with FastStream out of the box if you need it.

@Zhenay
Copy link
Contributor Author

Zhenay commented Aug 24, 2024

@Tishka17 Thank you for your suggestion, but we are completely satisfied with FastDepends.
I've heard about Dishka, it's a great and really powerfull tool. But in our case the most important criterion is simplicity and similarity with FastApi DI.

Lancetnik added a commit that referenced this pull request Sep 5, 2024
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

Successfully merging this pull request may close these issues.

3 participants