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

Issues using a deep-copied GraphQLSchema #100

Closed
pmantica1 opened this issue Jul 7, 2020 · 3 comments
Closed

Issues using a deep-copied GraphQLSchema #100

pmantica1 opened this issue Jul 7, 2020 · 3 comments
Assignees
Labels
investigate Needs investigaton or experimentation

Comments

@pmantica1
Copy link

pmantica1 commented Jul 7, 2020

To make our tests in the GraphQL compiler library run faster, we wanted to parse and build our test schema once and have each test case use a deep copied version of our test schema. We want each test to use their own deep-copied schema so that accidental schema modifications in one test case wouldn't affect the results of another test case.
kensho-technologies/graphql-compiler#873

It seems, however, that we can't use deep-copied schemas with certain GraphQL core functions as evidenced by the code below.:

from graphql import build_schema, print_schema, lexicographic_sort_schema
from copy import deepcopy

SCHEMA_TEXT = """
    schema {
        query: RootSchemaQuery
    }

    type Animal {
        uuid: ID
    }

    type RootSchemaQuery {
        Animal: [Animal]
    }
"""
# This test case passes
assert (
    print_schema(lexicographic_sort_schema(build_schema(SCHEMA_TEXT))) ==
    print_schema(lexicographic_sort_schema(build_schema(SCHEMA_TEXT)))
)

graphql_schema = build_schema(SCHEMA_TEXT)
graphql_schema_copy = deepcopy(graphql_schema)

# This test case fails. I included part of the stack trace as an image.
assert (
    print_schema(lexicographic_sort_schema(graphql_schema)) ==
    print_schema(lexicographic_sort_schema(graphql_schema_copy))
)

I think I figured out the problem with using a deepcopy in this particular instance.

The GraphQLSchema __init__ function uses a TypeSet that checks if two GraphQL types are equal by comparing their python object references. The __init__ function includes the builtin __Schema type is not already included in this TypeSet.

When we use deepcopy we generate new object references for all of the copied objects. So when we use the types in a deep-copied schema to build a lexicographically sorted schema, we call the GraphQLSchema __init__ function which doubly adds the __Schema type since the deep-copied __Schema type has a different reference than the original __Schema type. This in turns leads to an error later in the __init__ function call when checking if there are multiple types with the same name.

I suspect that we might find similar issues when using other GraphQL core functions. We, however, understand that making GraphQLSchema objects "deep-copyable" might take a lot of upfront work and a lot of maintenance work thereafter. So we understand if you don't want to commit to this. We still wanted to bring this issue to light though.

As always, thanks for the amazing work of porting the GraphQL js library to python.

Screenshot from 2020-07-07 09-53-09

@Cito
Copy link
Member

Cito commented Jul 7, 2020

Thanks for bringing this up. I will definitely look into this when I find some time.

@Cito Cito self-assigned this Jul 7, 2020
@Cito Cito added the investigate Needs investigaton or experimentation label Jul 7, 2020
@Cito
Copy link
Member

Cito commented Feb 7, 2021

Turned out to be a bit complicated, but I think it is now solved in 8c053a3.

Will be available in the upcoming release (soon).

@Cito Cito closed this as completed Feb 7, 2021
@Cito
Copy link
Member

Cito commented Feb 8, 2021

Version 3.1.3 with this fix has now been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Needs investigaton or experimentation
Projects
None yet
Development

No branches or pull requests

2 participants