import side-effect disables promise trampoline breaking DataLoaders

[AllexVeldman]

On import channels_graphql_ws, promise.promise.async_instance.disable_trampoline() is called, breaking DataLoaders.

In the code there is a reference to syrusakbary/promise#57 (comment) which indicates this is needed due to Promise not being thread-safe.

This should no longer be needed with promise ^2.3
syrusakbary/promise#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.

[prokher]

@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

[tony]

@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)

[matclayton]

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

[tony]

@matclayton Thank you for this!

[malthejorgensen]

For what it's worth, our (@tony and I) issue was this: graphql-python/graphql-core-legacy#287

[adrienhongcs]

Are there any plans to remove promise.promise.async_instance.disable_trampoline() in the next release?