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

Enhance docs for DataLoader & django 4.0+ async examples #1390

Open
martinschnurer opened this issue Feb 21, 2023 · 11 comments
Open

Enhance docs for DataLoader & django 4.0+ async examples #1390

martinschnurer opened this issue Feb 21, 2023 · 11 comments

Comments

@martinschnurer
Copy link

martinschnurer commented Feb 21, 2023

I went through docs for dataloader & django execution examples, but they seem not working with django 4+ anymore. Django suggests using sync_to_async ( and async_to_sync) wrappers, but I can't wrap my head around that for now. It would be really cool to see some more examples on running django v4 and dataloader (aiodataloader).

Versions

aiodataloader==0.3.0
Django==4.2b 
graphene==3.2
graphene-django==3.0.0
graphene-file-upload==1.3.0
graphql-core==3.2.3
graphql-relay==3.2.0

What I have

from graphene_django import DjangoObjectType
from aiodataloader import DataLoader

class PhotoFileByImageIdDataLoader(DataLoader):
    cache = False

    async def batch_load_fn(self, image_ids):
        images = Image.objects.filter(id__in=image_ids).select_related("photo_file")

        photo_file_by_image_ids = {image.id: image.photo_file for image in images}

        return [photo_file_by_image_ids.get(image_id) for image_id in image_ids]


loader = PhotoFileByImageIdDataLoader()

class ImageNode(DjangoObjectType):
    ...
    async def resolve_photo_file(self, *args, **kwargs):
        return await loader.load(self.id)

What I'm getting after execution:

 {
   "errors":[
      {
         "message":"Received incompatible instance \"<coroutine object ImageNode.resolve_photo_file at 0x7f6cb7820270>\".",
         "locations":[
            {
               "line":26,
               "column":7
            }
         ],
         "path":[
            "feed",
            "edges",
            0,
            "node",
            "images",
            "edges",
            0,
            "node",
            "photoFile"
         ]
      },
   ...
  ],
 "results": [ ... ]
}

Important information: in JSON results, there were results, but photo_file field remains null on each object - that's where the problem is with the resolve_photo_file method I suppose.

I followed docs at https://docs.graphene-python.org/en/latest/execution/dataloader/

Have anyone stumbled upon something similar? I'm having hard times getting this work

@pfcodes
Copy link

pfcodes commented Apr 4, 2023

+1

1 similar comment
@firaskafri
Copy link
Collaborator

+1

@firaskafri
Copy link
Collaborator

firaskafri commented Apr 21, 2023

PR #1394 will address the async support!

@firaskafri
Copy link
Collaborator

PR #1394 to support async is ready for review!

@alexandrubese
Copy link

alexandrubese commented Jun 11, 2023

Guys,
I am a bit confused about how DataLoaders should work with graphene-django 3.
I've tried the approach from the documentation and I get a "Query.resolver coroutine was never awaited".

Are we in a situation where DataLoaders don't work in any way and they are just there in the documentation? Or do I need to use a ASGI server for them to work ? There is no mention in the documentation about this. The async/await examples are only found on the DataLoaders page without any mention on initial setup for them to work.

In the meantime I found this package that works really well (That is also mentioned in the Ariadne graphql library): https://github.com/jkimbo/graphql-sync-dataloaders

That gives you support for graphene v3 + the new graphql core that removed Promise based dataloaders support.

What is the graphene-django's v3 way to use DataLoaders in a sync manner?

Can someone explain the situation more? Or what needs to be done for the examples from the docs to work? Do we need to wait for #1394 ?

Also for people that run Django in a sync manner (and don't plan to move all their queries to async and wrap everything in sync_to_async and async_to_sync) I think it's also a good idea to mention the sync dataloader package above in the DataLoader docs page, just so that people know about it and don't want to move to Async Django.

Ariadne are mentioning it, so it should be good to use.

@jkimbo What do you think ?

Thanks

@jpmrs1313
Copy link

Any updates on this?

@pfcodes
Copy link

pfcodes commented Dec 21, 2023

Bump

3 similar comments
@rw88
Copy link

rw88 commented Jan 10, 2024

Bump

@danihodovic
Copy link

Bump

@elyas-hedayat
Copy link

Bump

@PoByBolek
Copy link

I just posted an answer to a similar question on Stackoverflow. My problem was that I couldn't even construct the DataLoaders, because there was no running event loop (because manage.py runserver starts a synchronous development server).

But I finally got a super small example working using an asyncio.Runner that creates a new event loop for every request:

class BadAsyncDataLoaderGraphQLView(GraphQLView):
    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.runner = asyncio.Runner()

    def get_context(self, request):
        context = super().get_context(request)
        event_loop = self.runner.get_loop()
        context.photo_loader = PhotoFileByImageIdDataLoader(loop=event_loop)
        return context

    def execute_graphql_request(self, *args, **kwargs):
        # this creates a new event loop for *every* request :(
        with self.runner:
            result = super().execute_graphql_request(*args, **kwargs)

            if hasattr(result, '__await__'):
                async def wait_for(value):
                    return await value

                result = self.runner.run(wait_for(result))

            return result


class ImageNode(DjangoObjectType):
    photo_file = ...

    @staticmethod
    async def resolve_photo_file(parent, info, *args, **kwargs):
        return await info.context.photo_loader.load(parent.id)

But waiting for #1394 getting merged is probably a safer option 😉.


Note that you probably have to decorate your batch_load_fn() method with @sync_to_async unless you're using Django's async queries or no queries at all:

class PhotoFileByImageIdDataLoader(DataLoader):
    cache = False

    @sync_to_async
    def batch_load_fn(self, image_ids):
        images = Image.objects.filter(id__in=image_ids).select_related("photo_file")

        photo_file_by_image_ids = {image.id: image.photo_file for image in images}

        return [photo_file_by_image_ids.get(image_id) for image_id in image_ids]

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

No branches or pull requests

9 participants