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

Add execution_context_class argument in Flask GraphQLView #77

Closed

Conversation

alvinchow86
Copy link
Contributor

@alvinchow86 alvinchow86 commented Dec 11, 2020

Proposed PR for issue posted in #76

Did a quick test and this works in my local environment

if this looks like something that makes sense, I can work on necessary changes to merge (tests and such)

Possible other changes

  • add this to the other server integrations?
  • maybe add some of the other arguments in execute(), or make a more generic way to be able to pass arbitrary **kwargs

Usage example

from graphene.types.schema import UnforgivingExecutionContext

view = GraphQLView.as_view(
       ...
        execution_context_class=UnforgivingExecutionContext,
    )

@KingDarBoja
Copy link
Contributor

KingDarBoja commented Dec 12, 2020

Changes looks great as it isn't a big issue with arbitrary kwargs being passed to run_http_query.

Under the hood, these graphql-server methods make use of graphql-core execute() function which supports specific arguments like as execution_context_class but are hidden inside graphql-server run_http_query method as **execute_options.

This change should be easy to implement on the other integrations as well but would like some test suite for this new feature.

@alvinchow86
Copy link
Contributor Author

Sounds good will look into adding tests

@jzelinskie
Copy link

Any way to help get this change across the finish line? This seems pretty critical to error handling.

@alvinchow86
Copy link
Contributor Author

Yeah sorry haven't had a chance to work on adding the tests. I'll see if I can find some time soon; if you'd like to help with that feel free to submit a commit and I can merge it here

@kiendang
Copy link
Collaborator

kiendang commented Apr 6, 2023

Superseded by #100

@kiendang kiendang closed this Apr 6, 2023
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.

4 participants