client.execute(gql, variables) passes variable_values multiple times

[deifactor]

Calling client.execute(gql, variables) causes an exception along the lines of:

TypeError: execute() got multiple values for argument 'variable_values'
[... most of stacktrace omitted]
  File "[PATH TO EMPLOYER CODE OMITTED]", line 60, in <lambda>
    lambda: self.gql_client.execute(gql(query), variables),
  File "gql/client.py", line 193, in execute
    return self.execute_sync(document, *args, **kwargs)
  File "gql/client.py", line 137, in execute_sync
    return session.execute(document, *args, **kwargs)
  File "gql/client.py", line 442, in execute
    **kwargs,
  File "gql/client.py", line 361, in _execute
    **kwargs,

I believe this is because in Client.execute the variables get bundled up into *args, and then SyncClientSession.execute passes the variable_values parameter both in *args and explicitly.

Passing the variables as a keyword argument like client.execute(gql, variable_values=variables) appears to work, though I'm not 100% sure.

System info (please complete the following information):

• OS: macOS 11.6
• Python version: 3.7
• gql version: 3.0.0
• graphql-core version: 3.2.0

[leszekhanusz]

Yes, you have to use variable_values as described in the documentation
You're not the first to report this apparently.
I'll add it in the release notes for now but that could be easily fixed by explicitly adding the argument in the client methods. I'll see what I can do.

[deifactor]

It's not clear to me from the documentation that it has to be passed by keyword value. IMO if a value has to be passed as a keyword argument then it should be marked as keyword-only.

[leszekhanusz]

In gql 2.x:

in client.py, we received all execute arguments only as *args and **kwargs and pass those arguments to the transport
in requests.py, the variable_values argument appears and there is no *args (variable_values is not a keyword-only argument)

So it was possible to set the variable_values argument without using the keyword.

In gql 3.x:

we had to process the variable_values in the client.py code because we now support serialization of variables for custom scalars.
But because there is a *args in the execute methods in client.py, variable_values now becomes a keyword-only argument by definition.

I actually checked if the optional positional arguments *args in client.py was actually used anywhere in our code and can confirm that it is not used in any of our transports (all tests are passing without it)

So now we have two options:

1. variable_values stays as a keyword-only argument for 3.x

OR

2. we remove the *args optional positional arguments in client.py and:
   • it is now possible to provide variable_values as a positional argument without the keyword
   • but it might break hypothetical transports made by users which required additional positional arguments (unlikely I think), additional keyword arguments are of course still possible with **kwargs

@Cito what do you think would be the best approach ?

[Cito]

I would remove the *args , but leave the *kwargs, and use the same signature for execute() in Client as in SyncClientSession and AsyncClientSession. Then you could still pass variable_values as positional parameter.