Recreate loader if old loader is on different loop#395
Recreate loader if old loader is on different loop#395erikwrede merged 2 commits intographql-python:masterfrom
Conversation
|
@erikwrede any way to get a review on this? Would love to hear if this matches your mental model of how the batching loaders should work. |
|
Hey @zeptonaut, please excuse the long wait on this issue. The last months have been very busy so this slipped through. The issues with dataloader and event loops are known and a better solution is definitely necessary 😊 I'll discuss your approach with the others in our discord meeting on monday @ 9PM CEST and will get back to you. |
|
Hi @erikwrede : for what it's worth, this is no longer blocking us as we figured out an alternate way to do data loaders. Ultimately, we ran into problems when we updated to a newer version of Graphene or Graphene SQLAlchemy (can't remember) and this patch started causing problems. I do think though that the general idea though in this PR holds and that, like you said, figuring out a better mapping between data loaders and event loops probably needs to happen. |
|
@zeptonaut can you share how you handle dataloading now? Always looking for some new, proven approaches. A QueryBuilder of some sort is definitely on our list for the future, due to the known inefficiencies with dataloading and coroutine state explosion in large queries. |
3 participants
Fix for the problem encountered #35 (comment)
This change makes it so that the batched data loader is recreated at field resolution time if the current event loop is different than the event loop for the loader.
Previously, the loaders were only created at GraphQL
Schemacreation time. In many systems (including FastAPI and Starlette'sTestClient), theSchemaobject is required in order to initiate the mount of the GraphQL routes. Thereafter, a new event loop is created (owned by the request handler).This means that, almost by definition, in many frameworks the initial loaders will be created on one event loop and used on a different event loop in the future. To resolve this problem, this PR makes it so that we only use the memoized loader if it was created on the same event loop as where the resolution is happening.