DjangoChannelsGraphqlWs
DjangoChannelsGraphqlWs copied to clipboard
datadvance
import side-effect disables promise trampoline breaking DataLoaders
On import channels_graphql_ws, promise.promise.async_instance.disable_trampoline() is called, breaking DataLoaders.
In the code there is a reference to https://github.com/syrusakbary/promise/issues/57#issuecomment-406778476 which indicates this is needed due to Promise not being thread-safe.
This should no longer be needed with promise ^2.3
https://github.com/syrusakbary/promise/pull/81
https://github.com/syrusakbary/promise/releases/tag/v2.3.0
For some reason not completely clear to me yet this only impacts my unittests, the async_instance during normal operation seems to have trampoline_enabled=True.
@alexmnazarenko Considering this thread-safe issue as fixed we can probably throw away invocation of disable_trampoline(). We need to be careful here, AFAIR it took significant about of time to debug this for the first time.
@matclayton
We're seeing the exact same issue, enable_trampoline() fixed it
Any example of where you (or anyone else who happens upon this issue) placed the enable_trampoline() code?
(I assume if @AllexVeldman is correct on promise not needing it, doing it as early as possible would be best)
For sure
class GraphQLConfig(AppConfig):
name = "mixcloud.core.graphql"
label = "graphql"
def ready(self) -> None:
# This is dangerous
# This is to counteract https://github.com/datadvance/DjangoChannelsGraphqlWs/blob/master/channels_graphql_ws/graphql_ws_consumer.py#L165
# DjangoChannelsGraphqlWs disables_trampoline, which by default prevents dataloaders and promises from working, t
# This would cause a LOT of tests to fail,
# We need to put this back to the original value after DjangoChannelsGraphqlWs has done the damage.
# These is a plan to remove this from the base lib https://github.com/datadvance/DjangoChannelsGraphqlWs/issues/67
# as promises 2.4+ is now thread safe, so their patch shouldn't be required anymore, enable_tramoline could be removed at that point.
import promise
promise.promise.async_instance.enable_trampoline()
We do this in an AppConfig, so its enabled at bootup
For what it's worth, our (@tony and I) issue was this: https://github.com/graphql-python/graphql-core-legacy/pull/287
Are there any plans to remove promise.promise.async_instance.disable_trampoline() in the next release?
