881: Corrected enum metaclass to fix pickle.dumps()

[senseysensor]

No description provided.

[senseysensor]

this is fix for issue #881.

[senseysensor]

wow, seems fix doesn't work well for python less than 3.11. Will check with lower versions.
Upd: this works for me well on python 3.10, seems this is some side-effect in how tox runs the tests. Will try to investigate this

[senseysensor]

update: this was not related to tox in any way, but that was case with colliding enum class names defined in different modules (in different tests).
Proposed solution is not ideal but I think that would cover most cases of pickling.

[erikwrede]

Thank you for your submission of the PR and for conducting the investigation! I find it reasonable to enforce the uniqueness of Enum Names, as type names within a GQL schema are inherently unique. I would appreciate it if you could clarify whether this restriction solely applies to graphene Enums or if both Graphene Enums and normal Enums can have the same name.

I am also interested in exploring other methods of enhancing pickle support. Would a solution like the following be effective in addressing the issue?

class EnumMeta:
	...
	def __reduce__(self):
	        return (self.__name__, tuple(self.__bases__), dict(self.__members__))

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (2da8e9d) 96.00% compared to head (b793ee8) 96.01%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1495   +/-   ##
=======================================
  Coverage   96.00%   96.01%           
=======================================
  Files          51       51           
  Lines        1753     1755    +2     
=======================================
+ Hits         1683     1685    +2     
  Misses         70       70           

Impacted Files	Coverage Δ
graphene/types/enum.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[senseysensor]

Well, first of all, enforcing uniquness (of graphene enums only) is simple to implement, one more check can be added in __new__ method to achieve this. In this case we don't need anything from my 2nd commit (dac4ba4). To be honest, this overcome from dac4ba4 is not reliable (will work only for some specific cases) and includes black magic from inspect module which in most cases smells. I probably will revert back to f5b5a14 but will assign unique name of the class (across other tests).

Also not sure should we put hard restriction in creation non-unique graphene Enums or not, because the only side effect from this is that pickling wouldn't work, which is not so wide-used feature, and all other functionality works just fine. On the other hand, if we don't, user potentially may receive weird errors while pickling.

Limitation of unique Enum classes goes only for graphene enums and doesn't affect native python Enums in any way.

About your last question regarding better enum support and use some other methods like __reduce__ or __reduce_ex__ – this won't work, unfortunately. That was my first thought here, but source of the issue is not that graphen enum itself is not pickable, it perfectly is (it's just wrapper around native Enums, which supports pickle). But cause is the way how graphene enums are created, enum's direct metaclass is dynamically created / modified and thus pickle fails found its type in graphene module during its inner preliminary checks.

So to sum this up, I will commit another changes which revert code back to commit 1, but will rename class to get it unique to make tests pass.

[senseysensor]

@erikwrede , should be something else get changed here or that could be merged..?

[erikwrede]

Hey, thanks for the patience.
I'm unsure about modifying globals in the Metaclass and need to do some research on it first. Could this cause naming conflicts? (e.g. when you have a non-graphene Enum MyEnum and then generate a graphene Enum also named MyEnum from it)

[senseysensor]

Well, I probably am not completely sure about your considerations about possible conflicts. At least in specific example you gave that shouldn't be any conflict. globals() refers not to metaclass but to this specific module (__dict__ of
graphene/types/enum.py).
Native Enum class has its metaclass defined in standard library enum.EnumType. It doesn't care about graphene Enum in any way (graphene encapsulates native one). The only trick is done here is adding reference of dynamically created metaclass ( hypotethical GrapheneEnum) to graphene/types/enum.py module so that pickle dump algorithm is not get confused during internal checks.
So the only conflict I can consider is when we create several graphene Enums with same name in scope of runtime. In that case 1st item we'd added into __dict__ of the module would get overridden with 2nd definition, and pickle would throw another error during dump (anyway that should affect only pickle behavior).