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

Error when trying to serialize a Python enum #73

Closed
akdor1154 opened this issue Jan 3, 2020 · 6 comments
Closed

Error when trying to serialize a Python enum #73

akdor1154 opened this issue Jan 3, 2020 · 6 comments
Assignees
Labels
investigate Needs investigaton or experimentation

Comments

@akdor1154
Copy link

When returning a Python Enum-type object, I get an exception:
graphql.error.graphql_error.GraphQLError: Expected a value of type 'MyEnum' but received: <MyEnum instance>

Reproducible example:

from graphql import GraphQLSchema, graphql_sync, GraphQLObjectType, GraphQLField, GraphQLEnumType
from traceback import TracebackException
from enum import Enum

class MyEnum(Enum):
    Yes = 'YES'
    No = 'NO'

schema = GraphQLSchema(
    query=GraphQLObjectType(
        name='rootquery',
        fields={
            'testenum': GraphQLField(
                GraphQLEnumType('MyEnum', MyEnum)
            )
        }
    )
)
query = '{ testenum }'
root = {'testenum': MyEnum.Yes}
result = graphql_sync(schema, query, {'testenum': MyEnum.Yes})

try:
    assert result.data == {'testenum': 'YES'}
except:
    for e in result.errors:
        print(''.join(TracebackException.from_exception(e).format()))
    raise
@Cito
Copy link
Member

Cito commented Jan 3, 2020

The error message is a bit confusing, because the GraphQL and Python enum types have the same name "MyEnum". The type 'MyEnum' in the error message refers to the GraphQL enum type, the <MyEnum instance> to the Python enum type . The values of the GraphQL type are the same as the values of the Python type used to define it, i.e. "YES" and "NO". The result will be one of the names of the GraphQL type, i.e. "Yes" or "No" (usually in GraphQL, you use uppercase names for these, but this is just a convention). So your example should look like this:

query = '{ testenum }'
root = {'testenum': 'YES'}
result = graphql_sync(schema, query, root)
assert result.data == {'testenum': 'Yes'}

or

query = '{ testenum }'
root = {'testenum': MyEnum.Yes.value}
result = graphql_sync(schema, query, root)
assert result.data == {'testenum': MyEnum.Yes.name}

The values correspond to what is used in the data on the Python GraphQL server, the names correspond to what shall be used in the results on the GraphQL client. GraphQL-core will automatically translate between names and values.

I plan to write a detailed blog article on enum types in GraphQL to make all of this a bit more clear.

@Cito Cito closed this as completed Jan 3, 2020
@akdor1154
Copy link
Author

Hey thanks for the response.
Ok I kind of get the design, but then maybe I should rewrite this as a feature request to auto resolve Python Enum values? :) At first thought it seems like a logical thing to add if you are going to auto infer GraphQL enum possibilities from Python enum possibilities anyway. Would you accept a PR to this effect?

The alternative (in the case where my data model is using PyEnums and I want to serve them as GQL enums) is adding a custom resolver for every Enum field isn't it? (I know I could generalize such a resolver, but still..)
Is there a usecase where people are providing a PyEnum for the GQL Enum declaration, but the above behaviour would not be desired?

Jarrad

@Cito
Copy link
Member

Cito commented Jan 6, 2020

It depends on your backend data. If it uses 'YES' and 'NO', the above approach would work.

But maybe you're right, and this feature should be re-examined.

So we need to investigate and clarify this question: When definining a GraphQLEnum via a Python Enum, should the values of the GraphQLEnum be the enum members (i.e. MyEnum.YES, MyEnum.NO) or the enum values (i.e. 'YES', 'NO').?

As far as I see, graphql-core 2 only allowed OrderedDicts when defining GraphQLEnums. In graphql-core 3 you can still use dicts, and they behave the same, but if you use enums, it takes the enum values as GraphQL values. If you want to use the members, you currently can pass {'YES: MyEnum.YES, 'NO': MyEnum.NO} as the values parameter to GraphQLEnum.

Maybe we can add an option or helper function that allows using enums in both ways. This could also help with backward compatibility. Keep in mind graphql-core is a low-level library used by others such as Graphene, and we cannot simply introduce such breaking changes.

I think I will investigate this a bit more when I will write the blog article. I particularly want to deal with the situation where you have a database backend. In this case, you may have a database (like PostgreSQL) which has a particular enum type, and an ORM (like SQLAlchemy) with its own enum type, which may or may not be based on a Python enum class. I want all of this to play together seamlessly.

Leaving this open as a reminder to re-examine and write that blog post.

@Cito Cito reopened this Jan 6, 2020
@Cito Cito self-assigned this Jan 6, 2020
@Cito Cito added the investigate Needs investigaton or experimentation label Jan 6, 2020
@akdor1154
Copy link
Author

akdor1154 commented Jan 7, 2020

Thanks for the consideration. For reference I am going to use the following GraphQLEnumType definition based on your workaround:
GraphQLEnumType('MyEnum', {v.value: v for v in MyEnum})
From my perspective it would make sense if this were the default behaviour when calling GraphQLEnumType('name', SomeEnumType), but you probably have a better idea of the usecases of other users of this project.

@leszekhanusz
Copy link

@akdor1154 it should be GraphQLEnumType('MyEnum', {v.name: v for v in MyEnum}) I think. Using the name instead of the value as the key (in your case it is the same).

I agree that it would make more sense like this but I understand that it is a breaking change and thus difficult to change without impacting a lot of users.

Maybe that could be planned in a future major version bump ?

Note: made some tests with enums in gql PR 256

@Cito
Copy link
Member

Cito commented Jan 13, 2022

Made this configurable now. With GraphQLEnumType('MyEnum', MyEnum, names_as_values=True) you use the names of the enum as values. With GraphQLEnumType('MyEnum', MyEnum, names_as_values=None) you use the enum mebers themselves for the internal representation. However, in the result you will always get the name, i.e. in this case Yes.

@Cito Cito closed this as completed Jan 13, 2022
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

3 participants