Path to 0.5.0 about graphql-core-legacy HOT 14 CLOSED

@syrusakbary We have been looking a lot into a parallel middleware in our app (on Python 2.7). I was planning on open sourcing them, but the one we currently use is a simple ThreadPool middleware (the gevent middleware turned out not to be compatible with our app because we use grpc). That has its own issues though because it actually is spinning off threads. So our latest approach would be to use Futures (grpc is also using futures), which has a nice backport to python 2.7: https://github.com/agronholm/pythonfutures as well with an additional package to make it then-able: https://github.com/dvdotsenko/python-future-then

Apart from the fact that this would be neater for our app (so I will already be slightly more biased towards a futures solution), I also do think it is the more pythonic solution and more and more projects (like grpc) do actually implement the futures interface. So this would in my opinion, generally make it easier to integrate with the parallel middleware.

Since this is something that we need in our app and it has been on our internal TODO, that might be something that I, or possibly one of my colleagues could tackle. But the earliest for me would be sometime towards end of next week.

cc @alamaison

from graphql-core-legacy.

I've ported a few more commits:

Add to unit tests to ensure test accepts both edits of enter and leave: graphql/graphql-js@d2c005a
in graphql-python/graphql-core#56

Add test for parseLiteral on ComplexScalar: graphql/graphql-js@b3a5127
in graphql-python/graphql-core#57

Extract completeListValue funciton from CompleteValue: graphql/graphql-js@ab77470
Extract completeLeafValue from CompleteValue: graphql/graphql-js@4c5904c
Extract completeObjectValue from CompleteValue: graphql/graphql-js@5a13648
Extract completeAbstractValue from CompleteValue: graphql/graphql-js@d484f31
Add invariant for unreachable condition: graphql/graphql-js@7a15a3f
in graphql-python/graphql-core#58

Nit: fix missing quote in error message: graphql/graphql-js@db0924a
is already fixed so doesn't need doing

from graphql-core-legacy.

@Globegitter I think using futures is reasonable given the python context.
The main goal for using Promises is simplify the executor classes for only have one that could work in any case (sync or async resolver resolution).

If we could achieve it in a simple way with futures I think would be a good idea to use it instead of Promises. (Seems could be completely possible by using python-future-then 😉 )

from graphql-core-legacy.

@sdcooke you did a great work!

from graphql-core-legacy.

@Globegitter after researching and trying to implement using only futures I realized that everything would work great if we "promesify" the Futures automatically and have the advantages of using Promises at the same time.

So we can have the nice approach of Promises, but allowing the developer to use either a Deferred or Future if they want to. The Promise approach will not force any way for choosing how will be the executor (if asyncio, concurrent.futures, a Thread pool and so on), but will be only a layer on top of that.

from graphql-core-legacy.

@syrusakbary That sounds like a quite good option. What exactly would 'promesifying' Futures in your opinion mean? Would this just be adding the .then interface or more than that? I suppose a .catch could be quite handy as well.

from graphql-core-legacy.

@Globegitter What I meant with promesifying Futures is wrapping each future in a Promise (will be done in the grapqhl library, with no extra effort from the developer).

Here is the related PR for Promises, if you want a take al look.
graphql-python/graphql-core#59

And here is the code for each executor... you might find useful too!
https://github.com/graphql-python/graphql-core/tree/features/promises/graphql/execution/executors

from graphql-core-legacy.

Thanks, that is amazing progress @syrusakbary! Also this might be part of fixing #41 but it seems in your PR to 0.4.14 you missed: graphql/graphql-js@3d27953

from graphql-core-legacy.

@Globegitter I skipped intentionally this as we already pass the originalError to the GraphQLError exception.
https://github.com/graphql-python/graphql-core/blob/master/graphql/core/execution/executor.py#L230

Maybe what is not as obvious and need to be improved is that this original error is in the stack attr of the GraphQLError.
https://github.com/graphql-python/graphql-core/blob/master/graphql/core/error.py#L15

from graphql-core-legacy.

@sdcooke any other port of the graphql-js commits will be welcome! 😉

from graphql-core-legacy.

I haven't disappeared! Just not finding enough time to get to it right now. I'm currently battling against relay connections in graphene!

from graphql-core-legacy.

All the changes are now in the master branch. Closing this issue :)

from graphql-core-legacy.

Wow that's amazing progress. Great job @syrusakbary! :)

from graphql-core-legacy.

This new graphql-core version will be pushed to PyPI soon.
I'm working in updating all external dependencies (aka graphene, graphql_relay, graphql-django-view, django-graphiql, flask-graphql) to support the new code structure and upload a new version for them too.

from graphql-core-legacy.