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

Refactor DSL code #169

Merged
merged 23 commits into from
Nov 21, 2020
Merged

Conversation

leszekhanusz
Copy link
Collaborator

@leszekhanusz leszekhanusz commented Nov 7, 2020

Refactoring of the DSL code.

  • The DSLSchema only needs a schema, a Client is no more needed.
  • new dsl_gql function to convert the DSL operations into a Document
  • this allows DSL queries to be executed in a session instead of a client (Solves How to reuse connection when using DSL? #138)
  • Added typing annotations
  • Added code documentation and sphinx docs (Solves Document the "dsl" module/language #82)
  • supports Subscriptions
  • fix nested input arguments
  • allow to set the alias directly in the select method
  • allow multiple operations in a document
  • allow to set the operation name

Simple example:

query = dsl_gql(DSLQuery(ds.Query.hero.select(ds.Character.name)))                                                            
result = client.execute(query) 

Adding type annotations
Replace selections generator by get_ast_fields function
Remove ast property
Replace serialize_list method by a lambda
Remove obsolete test_nested_input_with_old_get_arg_serializer test
Move get_arg_serializer function inside the DSLField class
Rename get_field method of DSLType to _get_field
DSLSchema now requires a schema instead of a client
query and mutate methods of DSLSchema have been replaced by a gql method
which will return a DocumentNode
Set get_ast_field as a staticmethod of the DSLField class
Move DSLSchema.gql method out of the class to the dsl_gql funciton
Add a test_multiple_queries test
Put the _get_field method of DSLType inside __getattr__
The GraphQL type is saved in each DSLField
This allows us to compute the operation directly from the fields
(no need for the operation argument anymore)

In dsl_gql, we check that all the fields have the correct type
and that they are root fields (Query, Mutation and Subscription)

Add new AttributeError for types which are not in the schema
Replacing the KeyError by an AttributeError in the __getattr__ method of DSLType

Adding debug log messages when creating fields and types
@codecov-io
Copy link

codecov-io commented Nov 7, 2020

Codecov Report

Merging #169 (a2d56d9) into master (5874f54) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #169   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          939       946    +7     
=========================================
+ Hits           939       946    +7     
Impacted Files Coverage Δ
gql/dsl.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 5874f54...a2d56d9. Read the comment docs.

@leszekhanusz
Copy link
Collaborator Author

I'll add documentation tomorrow before merging

@leszekhanusz leszekhanusz added the type: feature A new feature label Nov 7, 2020
Rename known_serializers to known_arg_serializers
Added __repr__ dunder methods to DSLType and DSLField
Added debug logs
gql/dsl.py Outdated Show resolved Hide resolved
gql/dsl.py Outdated Show resolved Hide resolved
gql/dsl.py Outdated Show resolved Hide resolved
Copy link
Contributor

@KingDarBoja KingDarBoja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outstanding work @leszekhanusz

Also it seems you also fixed #104, right?

Adding executable examples for async and sync clients
Added to README.md features
@leszekhanusz
Copy link
Collaborator Author

Outstanding work @leszekhanusz

Thanks 😄

Also it seems you also fixed #104, right?

No, this is not related, #104 is not using the DSL module at all.

gql/dsl.py Outdated Show resolved Hide resolved
Copy link
Contributor

@KingDarBoja KingDarBoja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great, although I have made the alias change to work locally and tested it without any issue, should I push the commit to this branch so everything gets merged on one shot?

Done 🍰

gql/dsl.py Outdated Show resolved Hide resolved
gql/dsl.py Outdated Show resolved Hide resolved
gql/dsl.py Outdated Show resolved Hide resolved
@leszekhanusz
Copy link
Collaborator Author

I can't believe it 🤦‍♂️ , the _get_arg_serializer function was not needed after all as all it did was to rewrite the ast_from_value method. It is now removed.

Copy link
Contributor

@KingDarBoja KingDarBoja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, I loved the latest changes and the operation name feature :D

@leszekhanusz leszekhanusz merged commit 44803d4 into graphql-python:master Nov 21, 2020
@leszekhanusz leszekhanusz deleted the refactor_dsl branch November 22, 2020 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants