Skip to content

Cache parsed queries by default #2

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

Closed
wants to merge 2 commits into from

Conversation

Hellzed
Copy link
Contributor

@Hellzed Hellzed commented Jan 5, 2018

This would need support in GraphQL server core, as per the following PR:
graphql-python/graphql-server#7

For simple queries, the main performance issue is the parsing stage, and people usually end up rewriting graphql-server-core (and aiohttp-graphql for people using it) to allow caching, logging etc.

Enabling configurable caching by default in aiohttp-graphql may be useful.

@dfee
Copy link
Member

dfee commented Jan 5, 2018

Thanks for submitting. Let's wait on @syrusakbary to review and resolve your referenced PR than we can move forward here.

Two next steps that will be required are to write a test for get_cached_parser and allow a user to opt in or out of using the cached parser (or their own version of this function).

I imagine something as simple as moving get_cached_parser out of the class and adding it either to a utils.py file, or to the graphql_server repo and providing the interface for it in this package.

Otherwise, if you want to break out the first commit for the GraphiQL update, those are more easily accepted.

Again, thanks for the submission!

@syrusakbary
Copy link
Member

@dfee @Hellzed I'm working in a pluggable backend/engine for GraphQL with different integrations across graphql servers (Flask, Django, aiohttp, ...).
This will permit to handle caching, GraphQL query stores, [...], in a much more transparent and pluggable way.

https://github.com/graphql-python/graphql-env

@Hellzed
Copy link
Contributor Author

Hellzed commented Feb 13, 2018

@syrusakbary @dfee I'm starting to use graphql-env with aiohttp (the server is a stripped down version of aiohttp-graphql that relies on a Django-like view, configured by storing the GraphQLEnvironment in the aiohttp app object) and a custom caching and logging backend.

It's a real improvement in clarity compared to my previous setup but I understand graphql-env is under development. Should I look into making it live in graphql-env/server alongside the flask integration? Or should I wait for the project to mature a bit?

@syrusakbary
Copy link
Member

GraphQL-env is completely safe to use in a production environment, and is the next step of proper execution of GraphQL queries.
However it's API it's a bit more open for improvement.

The end goal is moving almost all the GraphQL server implementations (Django view, Flask, aiohttp) to use GraphQL-env under the hood.

@syrusakbary
Copy link
Member

Sorry I didn't answer your reply: if you can create a PR into GraphQL env that would be great, it will help to integrate GraphQL-Env into this repo later :)

@Hellzed
Copy link
Contributor Author

Hellzed commented Feb 14, 2018

Ok, I'll start work on moving my aiohttp based server into graphql-env.

I'll have two smaller PRs though (putting this here partially as a note to self):

  • exporting GraphQLParams as part of the top level API since it helps building third party servers;

  • making the data argument optional in params_from_http_request(query_params, data) since HTTP requests in our case cover both GET and POST requests, and GET requests' bodies should be ignored even if present, so there will be no data to provide in that use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants