Skip to content

Add execution_context_class argument in Flask GraphQLView #77

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.

Sign up for GitHub

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

Jump to bottom
Closed
alvinchow86 wants to merge 1 commit into from
Closed

Add execution_context_class argument in Flask GraphQLView #77

alvinchow86 wants to merge 1 commit into from

Conversation

@alvinchow86
Copy link
Contributor

@alvinchow86 alvinchow86 commented Dec 11, 2020 •
edited
Loading

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 •
edited
Loading

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.

@KingDarBoja KingDarBoja mentioned this pull request Dec 12, 2020
Closed
@alvinchow86
Copy link
Contributor Author

alvinchow86 commented Dec 15, 2020

Sounds good will look into adding tests

@jzelinskie
Copy link

jzelinskie commented May 2, 2022

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

@alvinchow86
Copy link
Contributor Author

alvinchow86 commented May 2, 2022

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 kiendang mentioned this pull request Dec 26, 2022
Merged
@kiendang
Copy link
Contributor

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alvinchow86 @KingDarBoja @jzelinskie @kiendang