Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Merge flask-graphql, sanic-graphql etc into graphql-server-core #34

Closed
4 tasks done
jkimbo opened this issue Mar 14, 2020 · 6 comments
Closed
4 tasks done
Labels
type: rfc Use this label for RFCs (request for comments) for broad, sweeping changes to graphql-server-core

Comments

@jkimbo
Copy link
Member

jkimbo commented Mar 14, 2020

The problem

Currently server integrations are split into separate repos:

And they are all depend on https://github.com/graphql-python/graphql-server-core

The issue with this is that each framework specific library shares a lot of code with the other libraries and it's hard to coordinate cross cutting changes.

The solution

The suggestion is that we merge the framework specific code into the grapqhl-server-core library so that the code is consolidated into 1 place. That way it's much easier to share code and keep things up to date.

To install the integration with flask for example:

pip install graphql-server-core[flask]

Then you can use it by:

from grapqhl_server.flask import GraphQLView

...

Drawbacks

It will make the graphql-server-core library harder to maintain. At the moment it is very small and easy to test. Merging in the other server libs will make the library become more complex, harder to test (you will need to install all of the dependencies for testing) and easier to accidentally add unneeded dependencies. Any contributions will have to make sure they don't break other server variants supported by the library which they are not familiar with.

Alternatives

We keep the separation of libraries and try and consolidate folder layout and coding standards.

List of packages to merge

@jkimbo jkimbo changed the title Suggestion: Merge flask-graphql, sanic-graphql etc into graphql-server-core RFC: Merge flask-graphql, sanic-graphql etc into graphql-server-core Mar 14, 2020
@jkimbo
Copy link
Member Author

jkimbo commented Mar 14, 2020

@KingDarBoja is going to make a first go at this with integrating flask-graphql into this library.

@KingDarBoja KingDarBoja added the type: rfc Use this label for RFCs (request for comments) for broad, sweeping changes to graphql-server-core label Apr 12, 2020
@KingDarBoja
Copy link
Contributor

KingDarBoja commented May 5, 2020

Alright, flask-graphql got merged into this, next one should be sanic-graphql.

@jnak
Copy link

jnak commented May 27, 2020

I'm definitely in favor of this change. I've identified improvements in flask-graphql that would entail touching all the different server integration repos.

@KingDarBoja
Copy link
Contributor

This is finally done! All four backend servers are merged into graphql-server-core.

@jkimbo
Copy link
Member Author

jkimbo commented Jul 5, 2020

Great work @KingDarBoja ! Going to close this issue now.

@Cito
Copy link
Member

Cito commented Jul 5, 2020

Time to rename this from "graphql-server-core" to probably just "graphql-server" now. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: rfc Use this label for RFCs (request for comments) for broad, sweeping changes to graphql-server-core
Projects
None yet
Development

No branches or pull requests

4 participants