Repository navigation
Support/help with promise-based resolvers #148
Description
I think I have a good use-case for non-async, promise-based resolution.
We are making django ORM from our dataloaders. We moved away from using async in django 3.0 because django would force us to isolate ORM calls and wrap them in sync_to_async. Instead, we ditched async and used promises with a generator based syntax. Examples below:
What we'd like to do, but django doesn't allow
class MyDataLoader(...):
async def batch_load(self, ids):
data_from_other_loader = await other_loader.load_many(ids)
data_from_orm = MyModel.objects.filter(id__in=ids) # error! can't call django ORM from async context.
# return processed combination of orm/loader dataWhat django would like us to do
class MyDataLoader(...):
async def batch_load(self, ids):
data_from_other_loader = await other_loader.load_many(ids)
data_from_orm = await get_orm_data()
# return processed combination of orm/loader data
@sync_to_async
def get_orm_data(ids):
return MyModel.objects.filter(id__in=ids)What we settled on instead (ditch async, use generator-syntax around promises)
class MyDataLoader(...):
def batch_load(self,ids):
data_from_other_loader = yield other_loader.load_many(ids)
data_from_orm = MyModel.objects.filter(id__in=ids)
# return processed combination of orm/loader dataI have a generator_function_to_promise tool that allows this syntax, as well as a middleware that converts generators returned from resolvers into promises. I have hundreds of dataloaders following this pattern. I don't want to be stuck isolating all the ORM calls as per django's recommendations because it's noisy and decreases legibility.
If it's not difficult to re-add promise support, I'd really appreciate it. If not, can anyone think of a solution to my problem?
Activity
Cito commentedon Nov 25, 2021
Since Python and its standard lib do not support promises, my first question is: What kind of promises do you have in mind? Is this something that Django provides? Or are you using the separate promise package used by Core-2?
AlexCLeduc commentedon Nov 25, 2021
Yes, we are using the promise package used by Core-2 and the dataloader class included with graphene 2.
Cito commentedon Nov 25, 2021
The problem with this is that these are legacy packages. I'm not sure how well they are maintained and I want to keep GraphQL-core simple and not in some way dependent of external non-standard packages or in the need of supporting these forever. The execution mechanism has also been completely changed and simplified, relying only on async.io, so no, it would not be easy to add the old promise support back in a way that does not degrade performance.
GraphQL-core should really be a core library as the name says - if you want to have support for non-standard async mechanisms such as promises, this should happen in a separate package that adds these capabilities to GraphQL-core. I think that would be the cleaner and more maintainable way to solve this. The
middlewareandexecution_context_classparameters of theexecutefunction should be usable as hooks allwing to implement this, but if necessary we could add more hooks. Could you imagine to create such a package?AlexCLeduc commentedon Nov 25, 2021
Thanks for the advice, I'll look into middleware and
execution_context_class. I've also followed up with graphene to see if they can re-add support for promises. You can close this issue if you'd like, but it might be a good place to discuss those additional hooks, should the need arise.jkimbo commentedon Nov 26, 2021
@Cito I think there is justification to add some kind of support for deferred functions that is not asyncio into graphql core because not all libraries are asyncio compatible yet (Django being the notable one). While it's possible to create a custom execution context class to handle Promises it's pretty fiddly and would be prone to breaking if anything changed. You can see my attempt here: strawberry-graphql/strawberry#614
I don't think graphql-core needs to support Promises specifically but having a way to handle deferred execution so that you can build dataloaders on threads instead of asyncio would be very helpful. GraphQL Ruby has the concept of lazy execution which is exactly what is needed: https://graphql-ruby.org/schema/lazy_execution
Cito commentedon Nov 26, 2021
@AlexCLeduc @jkimbo Thanks for your feedback. I understand that deferred execution (particularly for the DataLoader mechanism) without asyncio is a real world use case that we should support in some way.
@jkimbo you have a point that adding this support in a separate package or on a higher level like Graphene or Strawberry has also its downsides. If it relies on internal details of GraphQL-core it would be prone to break with every new release of GraphQL-core. To avoid this, we could either extend the official API (as suggested above) or actually move such support into GraphQL-core itself.
If we do this, then I think we agree it should be done in a generic way, not special casing for the old Promise lib or Django, and it should not degrade performance for the existing execution modes. Currently I don't have the time and need to work on this myself, so the initiative must come from people like you. Don't hesitate to send in PRs with suggestions. But please always include tests and performance benchmarks (see the tests/benchmarks directory).
I will keep this issue open for discussion and coordination of these efforts.
syrusakbary commentedon Feb 7, 2022
Hi, Syrus here!
I created the promises package a while ago. It's great for usage within synchronous contexts, however its usage is discouraged when you can just use
async/awaitfrom the syntax. Why? It adds significant overhead and the.thensyntax can make the code harder to understand (vsawait).It's true that it's quite painful go through the
sync_to_asyncfrom Django, but I'd say that this is an issue mainly on the Django side because their lack of async support.Personally, I agree that is ideal to keep promises abstraction "outside" GraphQL-core 3 as @Cito commented :)
Note:
Promisedoes a few tricks to allow dataloaders in synchronous contexts. I do think it will be quite hard to replicate that in graphql-core without reimplementing the same things. That means that theDeferredValue(from the PR #155) may not work as expected with Dataloaders (in a nested context).superlevure commentedon Jun 23, 2022
@syrusakbary thank you for the awesome
aiodataloader.I'm still unclear if it can work together with Django though ? (see this thread too: graphql-python/graphene#1391)
ericls commentedon Oct 4, 2022
Hi @superlevure we were facing the same issue and rewrote the executecontext in graphql-core to be promise based to support promised based resolvers. See it here: https://github.com/fellowapp/graphql-core-promise
ericls commentedon Oct 4, 2022
I kind of hope the execution context in this can be merged as part of graphql-core or something more official. @jkimbo also built a
DeferredExecutionContextin his https://github.com/jkimbo/graphql-sync-dataloaders project. I wonder if we should have an "official" thing to host the code for these execution contexts?It looks like all these context are just translations of the default execute context but with different "flavors", is it possible to write the execution context in a way that's runtime/scheduler agnostic, that allows users to choose their own runtimes?
Cito commentedon Oct 4, 2022
Something like this will eventually be integrated in GraphQL-Core, see discussion #155. Promises however will not be supported in GraphQL-Core 3 because they are not part of standard Python and because the only existing library for these is not maintained any more.
ericls commentedon Oct 4, 2022
That's good to hear! Do you know if the integrated solution will be based on something like https://github.com/jkimbo/graphql-sync-dataloaders/blob/main/graphql_sync_dataloaders/sync_future.py or is it going to be based on
asyncio.Futureorconcurrent.futures.Futureor something custom? Or if it's gonna be more generic and supports anything that implements a "future"-like protocol?Cito commentedon Oct 4, 2022
Need to find some more time to investigate and decide. But if you have suggestions, you can comment on #155.
patrys commentedon Dec 27, 2022
I can provide another use case: we want to migrate Saleor (a rather large codebase with a huge API) from Graphene 2.x (and GraphQL Core 2.x) to version 3. To do so, we will need to rewrite almost the entire codebase in one step, as there is no way to have promises running side-by-side with asyncio and rewrite dataloaders and resolvers one by one. Having a temporary way to run both would make our team (and I imagine anyone forced to migrate) very happy.