reagento / dishka

Cute DI framework with agreeable API and everything you need
https://dishka.readthedocs.io
Apache License 2.0
389 stars 40 forks source link

Unclear approach for integrating Dishka with Strawberry GraphQL #191

Closed koufopoulosf closed 2 months ago

koufopoulosf commented 2 months ago

Context

I'm trying to integrate Dishka with Strawberry GraphQL in a FastAPI application. The main challenge is injecting dependencies into GraphQL resolvers while maintaining compatibility with Strawberry's type system.

Working Solution

What ended up working was modifying how we pass the container to the GraphQL context:

async def get_context(request: Request) -> GraphQLContext:
    return GraphQLContext(request=request, container=request.state.dishka_container)

graphql_app = GraphQLRouter(schema, context_getter=get_context)

POC Available

To make it easier to get started with Strawberry, I put together a small POC.

Questions

  1. Is accessing the container through request.state.dishka_container the recommended approach for integrating Dishka with Strawberry GraphQL?
  2. Are there any potential issues or drawbacks with this method that we should be aware of?
  3. Is there a more idiomatic way to integrate Dishka with Strawberry GraphQL that we might have overlooked?
  4. Could the documentation be updated to include a specific section on integrating Dishka with GraphQL libraries like Strawberry?

Any guidance or best practices for this integration would be greatly appreciated.

Thank you for your time and for creating this wonderful library!

Tishka17 commented 2 months ago

Usage of request.state.dishka_container in fastapi application is a proper way to access to container outside of fastapi handlers. inject decorator uses it as well. Writing your own version of such decorator is up to you.

I am not sure what to add, except maybe details about fastapi and other existing integrations details

koufopoulosf commented 2 months ago

@Tishka17 Thank you for your quick response.

I discovered this solution by examining the setup_dishka code, as I couldn't find it in the documentation. I want to ensure that using request.state.dishka_container is the correct approach and not an internal detail we shouldn't rely on.

Two key requests:

  1. Could you please add this approach to the official documentation, especially for integrating Dishka with GraphQL libraries like Strawberry?

  2. Could you please review the simple POC and confirm if this implementation (using request.state.dishka_container) is the correct, long-term supported approach?

Your clarification on these points would be greatly appreciated. Thank you for your valuable work on this library.

Tishka17 commented 2 months ago

Approach depends on framework you are using. For fastapi container is stored inrequest.state. it is not planned to change unless fastapi breaks something

I'll review poc later.

Tishka17 commented 2 months ago

Your POC looks valid for me. I don't think you need components here (it is primarily needed for isolation of providers), but it is still ok if you want to set them

koufopoulosf commented 2 months ago

The proof of concept (POC) I shared is a simplified example demonstrating request.state.dishka_container in action, alongside Strawberry. In my actual project, which follows the same principles (including the use of components), I've observed some notable performance improvements. For example:

  1. There's a minor delay (a few milliseconds) on each request for resolving dependencies.
  2. Importantly, this doesn't negatively impact the overall requests per second.
  3. For concurrent requests, the library appears to manage resources more efficiently, even reducing latency in some cases.

I'll admit that I don't fully understand the underlying mechanisms of this library, but its effectiveness is undeniable. It's arguably the most impressive library I've encountered recently.

The most significant advantage is that it allows me to focus on development rather than wrestling with dependency management.

Thank you for your valuable contribution to this project. I wish you continued success in your development efforts!

koufopoulosf commented 2 months ago

@Tishka17 I closed the issue for now. If you feel you could add a section on documentation or an example about FastAPI + Strawberry, that would be really great.

Last, if you think there are any other good practises I should follow / be aware of, please enlighten me 😄

Thank you once again 🙏