FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add execution_context_class argument in Flask GraphQLView by alvinchow86 · Pull Request #77 · graphql-python/graphql-server · GitHub

/ graphql-server Public

Add execution_context_class argument in Flask GraphQLView - #77

Closed
alvinchow86 wants to merge 1 commit into
graphql-python:masterfrom
alvinchow86:execution-context
ConversationCommits (1)ChecksFiles changed
Closed

Add execution_context_class argument in Flask GraphQLView#77
alvinchow86 wants to merge 1 commit into
graphql-python:masterfrom
alvinchow86:execution-context

Conversation

alvinchow86 commented Dec 11, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 commented Dec 12, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

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 mentioned this pull request Dec 12, 2020
Closed

alvinchow86 commented Dec 15, 2020

Copy link
Copy Markdown
Contributor Author

Sounds good will look into adding tests

jzelinskie commented May 2, 2022

Copy link
Copy Markdown

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

alvinchow86 commented May 2, 2022

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

kiendang commented Apr 6, 2023

Copy link
Copy Markdown
Contributor

Superseded by #100

kiendang closed this Apr 6, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
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

Back | FazBrowse Home | New Git URL