Add support for variable definitions in dsl

[wallee94]

Hi all,

I was working in a small project and I needed a way to compose queries dynamically with variable definitions, so I can create the queries once depending on initial settings and just change the variables when sending them.
I created a DSLVariableDefinitions and DSLVariable class in the dsl module to add the option. I tried to make it similar to the current API 🙂

This is an example of how it works:

var = DSLVariableDefinitions()
op = DSLMutation(
    ds.Mutation.createReview.args(review=var.review, episode=var.episode).select(
        ds.Review.stars, ds.Review.commentary
    )
)
op.variable_definitions = var
query = dsl_gql(op)

And it generates:

mutation ($review: ReviewInput, $episode: Episode) {
  createReview(review: $review, episode: $episode) {
    stars
    commentary
  }
}

I have no experience with sphinx, please let me know if I need to adjust something.

[leszekhanusz]

Thanks! I'll look into it.

[codecov-commenter]

Codecov Report

Merging #210 (8aef7bc) into master (4528977) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #210   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           16        16           
  Lines          985      1041   +56     
=========================================
+ Hits           985      1041   +56     

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 4528977...8aef7bc. Read the comment docs.

[leszekhanusz]

Your solution does not work with variables in input objects like this:

createReview(                                                                                                         
    review: {stars: $stars, commentary: $commentary}                                                                    
    episode: $episode                                                                                                   
  )

To solve this, the only way is to modify the recursive ast_from_value function from graphql-core.
As this function cannot be modified directly in graphql-core because it is a one-to-one copy of astFromValue from graphql-js
I made a copy paste inside gql. I agree that this is quite hackish.

graphql-js itself is currently in the process of changing quite a few things (See graphql-js PR #3049) and changing the astFromValue function to ValueToLiteral

I̶'̶l̶l̶ ̶m̶a̶k̶e̶ ̶a̶n̶ ̶i̶s̶s̶u̶e̶ ̶o̶n̶ ̶g̶r̶a̶p̶h̶q̶l̶-̶j̶s̶ ̶s̶i̶d̶e̶ ̶t̶o̶ ̶d̶o̶ ̶t̶h̶i̶s̶ ̶a̶ ̶l̶i̶t̶t̶l̶e̶ ̶b̶i̶t̶ ̶c̶l̶e̶a̶n̶e̶r̶.̶

[wallee94]

Oh, you're right! I missed that option.

I agree that the only workaround is going to be copying the ast_from_value function. It is hackish but at the same time I think it makes sense to have a custom version of ast_from_value in the dsl module, because returning VariableNodes is very related to composing a query.
It looks like valueToLiteral will add support for a valueToLiteral method in GraphQLInputType, but that wouldn't help because DSLVariables should work with any type

[leszekhanusz]

Looks good!

I added some docs.

Now for the hard part, could you please add tests so that the code coverage stays at 100% ?

Note: if you want to avoid linting issues, you can run make check and that should automatically solve them.

[wallee94]

Great! Thank you @leszekhanusz.
Sure, I'll work on the tests

[wallee94]

I added the tests and I think coverage is back to 100% again 🙂

[leszekhanusz]

Great!

[leszekhanusz]

LGTM

[leszekhanusz]

Thanks again for your PR!