feat: flask asyncio support for dataloaders

[wakemaster39]

Resolves #65

This takes the approach that no one should already have an event loop and makes one and runs it in context. This is also only python 3.7+ supported because of the use of asyncio.run. I am not sure if the run logic should be backported into here so it works on 3.6 as well.

[KingDarBoja]

This should be replaced by a call to previous result_results method, to avoid repeating code ;)

[KingDarBoja]

I don't like the idea of creating the event loop inside the execution step as not every user uses asyncio for that, other alternatives could be trio as example.

[KingDarBoja]

Th correct approach should be the same as sanic and aiohttp integrations does:

execution_results, all_params = run_http_query(
                self.schema,
                request_method,
                data,
                query_data=request.args,
                batch_enabled=self.batch,
                catch=catch,
                # Execute options
                run_sync=not self.enable_async,
                root_value=self.get_root_value(),
                context_value=self.get_context(),
                middleware=self.get_middleware(),
            )
            exec_res = (
                [
                    ex if ex is None or isinstance(ex, ExecutionResult) else await ex
                    for ex in execution_results
                ]
                if self.enable_async
                else execution_results
            )

[KingDarBoja]

Looking deeper into Flask itself, adding async capabilities seems to be a PITA as the framework itself does not support (AFAIK) async tasks by itself. However I found this alternative that seems to resemble Flask pattern: https://gitlab.com/pgjones/quart.

This is even recommended from the Pallets Team as stated on this issue: pallets/werkzeug#1322 (comment)

Cheers!

[wakemaster39]

@KingDarBoja you are 100% right that flask does not use asyncio and more specifically does not support ASGI. But this PR isn't about supporting ASGI, it is about supporting async calls so dataloaders can work.

ASGI is full async parallel calls craziness and that is honestly not something I am interested in or care to use. (This is my understanding of it at least). What this does is creates an event loop in the context of a single WSGI request and allows you to use async/await calls in that request. The request itself is still blocking.

I had to move away from this for a bit but I am going to be resuming it again next week. I am good if it is decided against allowing the creation of the event loop. But can we open up a hook inside of the call chain where someone else can with minimal work? With the removal of Promises, the original dataloaders library no longer works, which has blown apart my application and this seemed like the better call than injecting promise resolution in the same spot.

I originally went the route of adding the creation of the event loop to make it easy on people and I wanted to make it a simple override of the function if you wanted to remove it as I doubt many people are using asyncio in flask right now because it doesn't make sense in a-lot of situations.

I'll play around with the code a bit more next week and clean it up some if you are good with supporting this to some extent.

[KingDarBoja]

I had to move away from this for a bit but I am going to be resuming it again next week. I am good if it is decided against allowing the creation of the event loop. But can we open up a hook inside of the call chain where someone else can with minimal work?

Yeah, that should be the way to go, like the enable_async flag behaviour is gonna be used slighty different than sanic or aiohttp does and should be documented as well. I will refactor it a bit and try to provide a method call like you did but also should be great to add some tests because the CI passed as result of what you did but we don't know if it would break the server under certain circumstances.

Still opened #68 to add support for Quart as well as it is not so complicated to support, and keep this PR for the dataloader request.

Cheers!

[wakemaster39]

@KingDarBoja I finally had the bandwidth to come back to this, its been far too long.

I cleaned up and minimized the changes. There is now a single hook point you can overload if you use async somewhere else in flask and setup your own thread. But out of the box it "just works"

[colelin26]

Hi there! Are there any updates on this? I tried using this in my Flask, graphene V3 application and it still does not work for aiodataloader. I got this error message:

Task <Task pending name='Task-16' coro=<ExecutionContext.resolve_field.<locals>.await_result() got Future <Future pending> attached to a different loop

It seems to me that asyncio.run tried to create a new event loop, but there already exists another event loop for this task due to the implementation of aiodataloader. A normal async resolver (non-dataloader) would work for this approach tho.

Problem fixed by constructing aiodataloader inside resolver

[fffergal]

I recently had to deal with this at work while upgrading graphene. Would you like some help getting this out? I can review/write code/update the docs. I copy pasted the graphql_server.flask.graphqlview module and made changes, but would like to upstream that if possible.

As one user, my preference is to not start the event loop inside a library. asyncio usage is often very opinionated, bespoke, and hard to unwind. I would rather this library return a coroutine and leave the event loop to users, with some examples in the docs for starting an event loop inside the user's Flask view (with asyncio.run) to get people started. For example I had a similar problem to coelin26 where I needed to init the DataLoaders within the same event loop. I imagine this will be a common thing as DataLoaders are the reason we need to move to async.

Please let me know if the maintainers are interested in some help to merge and release this update! Thank you.

[kiendang]

@fffergal I agree that starting an event loop inside the library is not ideal. If you're still interested in working on this and submitting a PR I'd be happy to review.

[kiendang]

Just opened #102 which implements this for Flask 2.

[fffergal]

@fffergal I agree that starting an event loop inside the library is not ideal. If you're still interested in working on this and submitting a PR I'd be happy to review.

I'm sorry, I don't work at the same place any more so I'm not using graphql-python at the moment. I did notice while using asyncio DataLoaders, queries weren't being grouped together anymore, so there may be more to getting asyncio DataLoaders working effectively, vs. just working.

[syrusakbary]

This is no longer needed, as is supported in main