profusion / sgqlc

Simple GraphQL Client
https://sgqlc.readthedocs.io/
ISC License
513 stars 85 forks source link

Optionally disable Variable name mangling and permit `ContainerType` types to be represented by `Variable` #207

Closed ryandvmartin closed 2 years ago

barbieri commented 2 years ago

@ryandvmartin would you mind elaborating on the need? Unlike types, that are mandatory to be exactly what the server defined in its schema, the variable names are completely client-side defined, so it's irrelevant if they are snake_case, camelCase or even human readable.

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2544201240


Totals Coverage Status
Change from base Build 2186990982: 0.0%
Covered Lines: 1556
Relevant Lines: 1556

💛 - Coveralls
ryandvmartin commented 2 years ago

Thanks for the feedback. I think overall I've solved my issues withs some additional understanding on the structure of the gql schema.

The types have a structure:

task

project
    tasks = types.Field(types.non_null(types.list_of(types.non_null("task"))))

task_arr_rel_insert_input
    data = types.Field(types.non_null(types.list_of(types.non_null("task_insert_input"))))

project_insert_input
    tasks = types.Field("task_arr_rel_insert_input")

And I misunderstood what was needed here for task_arr_rel_insert_input when I attempted to do this on a project insert:

op.insert_project_one(
    object=dict(
        tasks=Variable("tasks")
    )
)

And this case failed with Variable is not a json object.. manually doing this with a string query + json I would have just passed {tasks: {data: [list_of_tasks]}} to match the structure. However, doing this:

op.insert_project_one(
    object=dict(
        tasks={'data': Variable("tasks")}
    )
)

with json {tasks: [list_of_tasks]} works.

On the renaming issue, our schema is snake-case, meaning that if I attempt to automate the json data passed to the request, I also have to convert the names I automatically pull off of the sgqlc types, e.g., if task has an attribute task.some_attribute and I pull that attribute into a Variable, its name gets converted on the Variable(attr) where attr="some_attribute", and then I have to manually convert that name to match in my json dictionary, e.g., json_data[convert(attr)] = data to make sure the names line up on the request. In this particular case I would just disable the conversion all together, and attaching that logic to the schema makes a LOT of sense! I'm not sure how to do that but that sounds like a great solution. For now I am creating the variable outside of the query dictionary, and then using the gql name on the json dictionary:

var = Variable(attr)
query_data[attr] = var
json_data[var.graphql_name] = data

This makes sure the variable's json data and the gql query have matching names regardless of any conversions.

Should close this PR unless you see any value in attaching some kind of "disable renaming" to the schema.

ryandvmartin commented 2 years ago

And this case failed with Variable is not a json object

Probably also a case that I got the types wrong when I set them up in the original operation..

barbieri commented 2 years ago

ok, got it! Yes, you can't use variables as part of a fixed payload, you just declare the whole payload (input type) as a variable.

As for the snake_case, got it. Your solution to use the name is the best, but if you want to provide a solution that disables the conversion I'd be ok, just make it per-schema instead of global.

With time you're going to notice in practice we may end talking to multiple GraphQL servers, then they may have different requirements and this would screw :-)