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 skip and include directive in introspection schema #279

Conversation

leszekhanusz
Copy link
Collaborator

Workaround for issue #278 until graphql-js#3419 has been fixed.

Always adding skip and include default directives in schema built from introspection.

@codecov-commenter
Copy link

Codecov Report

Merging #279 (d9b85de) into master (2be6aaa) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #279   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        23    +1     
  Lines         1800      1815   +15     
=========================================
+ Hits          1800      1815   +15     
Impacted Files Coverage Δ
gql/client.py 100.00% <100.00%> (ø)
gql/utilities/__init__.py 100.00% <100.00%> (ø)
gql/utilities/build_client_schema.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2be6aaa...d9b85de. Read the comment docs.

@Cito
Copy link
Member

Cito commented Dec 9, 2021

I agree that's the cleanest workaround, @leszekhanusz. Strictly speaking there are two more directives (deprecated and specifiedBy, the latter maybe only since GraphQL-core 3.2).

@leszekhanusz
Copy link
Collaborator Author

I agree that's the cleanest workaround, @leszekhanusz. Strictly speaking there are two more directives (deprecated and specifiedBy, the latter maybe only since GraphQL-core 3.2).

Yes I know but deprecated and specifiedBy are directives for the schema. I don't think they make sense inside a query.
As we are only validating queries in gql I think it could be enough.

@Cito
Copy link
Member

Cito commented Dec 9, 2021

Right, that makes sense.

@leszekhanusz leszekhanusz merged commit 09e4b30 into graphql-python:master Dec 10, 2021
@leszekhanusz leszekhanusz deleted the fix_workaround_for_missing_include_directive branch March 12, 2022 12:41
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