GraphQL schemas do not seem to be deep copyable

[pmantica1]

When I tried to optimize tests by using the deep copies, I got a ton of different errors. I thought that this was because of a compiler bug but it seems that this deepcopy somehow messes up the internals of GraphQLSchema.

While testing locally, test_normal_schema_comparison passed but test_deep_copy_comparison did not.

[obi1kenobi]

This seems like a good candidate for opening an issue in `graphql-core`. Before that, just check whether things like custom directives and custom scalars have anything to do with it, since I think you already mentioned that custom scalars have some surprising behavior to them.

…

On Sun, Jul 5, 2020, 1:48 PM pmantica1 ***@***.***> wrote: When I tried to optimize tests by using the deep copies, I got a ton of different errors. I thought that this was because of a compiler bug but it seems that this deepcopy somehow messes up the internals of GraphQLSchema. While testing locally, test_normal_schema_comparison passed but test_deep_copy_comparison did not. ------------------------------ You can view, comment on, or merge this pull request online at: #874 Commit Summary - GraphQL schemas do not seem to be deep copyable - Nit File Changes - *M* graphql_compiler/tests/test_schema.py <https://github.com/kensho-technologies/graphql-compiler/pull/874/files#diff-627ff9a1a2bf4b354ebf5611cf5de3fb> (17) Patch Links: - https://github.com/kensho-technologies/graphql-compiler/pull/874.patch - https://github.com/kensho-technologies/graphql-compiler/pull/874.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#874>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAR5MSWGWLVPRAB5AUQDM23R2C4FDANCNFSM4OQ7DGXQ> .

[pmantica1]

For the explanation below, please note that I now use a reduced version of the schema in this PR.

Ok so I think I figured out the problem or at least part of the problem with deepcopy.

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.

I suspect that other functions in the GraphQL core library might also refer to GraphQL types by their python object references. Unless the GraphQL library is patched to only refer to GraphQL types by their names, we are going to keep finding weird bugs like this one if we use a deep-copied schema.

I am going to submit an issue in the GraphQL core library. This refactor, however, probably won't be easy to make and might not even be wanted by maintainer. So I think that we'll probably want to go a different route for performance optimizing our tests. We should probably wait for the maintainer to respond to the issue though.

[pmantica1]

Made an issue:
graphql-python/graphql-core#100

[pmantica1]

Closing this PR since it doesn't really need review.