graphql-python / graphene

GraphQL framework for Python
http://graphene-python.org/
MIT License
8.06k stars 822 forks source link

Propsal: Add registry for enum conversion #1418

Closed erikwrede closed 2 months ago

erikwrede commented 2 years ago

Graphene offers a convenient method to convert a Python-Enum (PyEnum) to a graphene-compatible enum (GEnum) using the types.Enum.from_enum() function. However, this function creates a new type each time it is called, even when called with the same PyEnum twice:

from enum import Enum as PyEnum
from graphene import Enum

class MyEnum(PyEnum):
    A = 1
    B = 2
    C = 3

GEnum = Enum.from_enum(MyEnum)

GEnum2 = Enum.from_enum(MyEnum)

print(GEnum == GEnum2)

graphene-sqlalchemy currently uses an enum registry to convert SQLAlchemy enums to corresponding graphene enums without duplication:

https://github.com/graphql-python/graphene-sqlalchemy/blob/0820da77d94d947e35325ba40eea96462bc890a7/graphene_sqlalchemy/registry.py#L58-L68

I propose that we backport this method to graphene for use with all python Enum types. I don't see a use-case for having two converted enums referencing the same underlying enum. For backward compatibility, the old function could remain in code, or the use of the enum registry could be enabled/disabled using optional arguments. This change would be helpful for the automatic conversion of python enums to graphene enums. It might prevent user confusion when two types with the same properties are created from an underlying enum.

Would work on a PR if this is desired. Let me know what you think!

flipbit03 commented 2 years ago

This would be very helpful and also help a lot in keeping our data models sane.

This would also help in packages like graphene_sqlalchemy, where we just landed the ability to type the return of a @hybrid_property for GraphQL consumption, and returning a Python enum in a @hybrid_property is currently not possible (it is possible to support it - but it will not be a sound implementation as the type will be generated multiple times) because of the characteristic @erikwrede mentioned. A registry mechanism could do away with all this churn.

Some things that we need to design and take into consideration as defensive measures, since this is a library:

(this is notably a very bad practice, but we need to be defensive about this stuff since we are a library) What happens in the user has 2 enums with the same name in different files, and they have differing invariants?