Repository: graphql-python/graphene
Pull request #1495 (2023-02-13 15:24:15)

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

senseysensor: wow, seems fix doesn't work well for python less than 3.11. Will check with lower versions.
senseysensor: update: this was not related to tox in anyway, 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 a 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?

```python
class EnumMeta:
	...
	def __reduce__(self):
	        return (self.__name__, tuple(self.__bases__), dict(self.__members__))
```
codecov[bot]: # [Codecov](https://codecov.io/gh/graphql-python/graphene/pull/1495?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python) Report
Base: **95.98**% // Head: **95.92**% // Decreases project coverage by **`-0.06%`** :warning:
> Coverage data is based on head [(`dac4ba4`)](https://codecov.io/gh/graphql-python/graphene/pull/1495?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python) compared to base [(`340d5ed`)](https://codecov.io/gh/graphql-python/graphene/commit/340d5ed12f7e736ca6ca6fd82c9ec4abdc635d4a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python).
> Patch coverage: 92.85% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1495      +/-   ##
==========================================
- Coverage   95.98%   95.92%   -0.06%     
==========================================
  Files          51       51              
  Lines        1742     1769      +27     
==========================================
+ Hits         1672     1697      +25     
- Misses         70       72       +2     
```


| [Impacted Files](https://codecov.io/gh/graphql-python/graphene/pull/1495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python) | Coverage Δ | |
|---|---|---|
| [graphene/types/enum.py](https://codecov.io/gh/graphql-python/graphene/pull/1495?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python#diff-Z3JhcGhlbmUvdHlwZXMvZW51bS5weQ==) | `97.50% <92.85%> (-2.50%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/graphql-python/graphene/pull/1495?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=graphql-python).

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 (https://github.com/graphql-python/graphene/pull/1495/commits/dac4ba4e1261526fb012955db7be0f5a4eb6b839). To be honest, this overcome from https://github.com/graphql-python/graphene/pull/1495/commits/dac4ba4e1261526fb012955db7be0f5a4eb6b839 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 https://github.com/graphql-python/graphene/pull/1495/commits/f5b5a14f3f35eb215b9592e456cb5018044ea39a 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 encapsulate 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 added into `__dict__` of the module will get overridden with 2nd definition, and pickle would throw another error during dump (anyway that should affect only pickle behavior).

Patch:
diff --git a/graphene/types/enum.py b/graphene/types/enum.py
index 7d68ccd..d3469a1 100644
--- a/graphene/types/enum.py
+++ b/graphene/types/enum.py
@@ -31,9 +31,11 @@ class EnumMeta(SubclassWithMeta_Meta):
         # with the enum values.
         enum_members.pop("Meta", None)
         enum = PyEnum(cls.__name__, enum_members)
-        return SubclassWithMeta_Meta.__new__(
+        obj = SubclassWithMeta_Meta.__new__(
             cls, name_, bases, dict(classdict, __enum__=enum), **options
         )
+        globals()[name_] = obj.__enum__
+        return obj
 
     def get(cls, value):
         return cls._meta.enum(value)


Test patch:
diff --git a/graphene/tests/issues/test_881.py b/graphene/tests/issues/test_881.py
new file mode 100644
index 0000000..f97b591
--- /dev/null
+++ b/graphene/tests/issues/test_881.py
@@ -0,0 +1,27 @@
+import pickle
+
+from ...types.enum import Enum
+
+
+class PickleEnum(Enum):
+    # is defined outside of test because pickle unable to dump class inside ot pytest function
+    A = "a"
+    B = 1
+
+
+def test_enums_pickling():
+    a = PickleEnum.A
+    pickled = pickle.dumps(a)
+    restored = pickle.loads(pickled)
+    assert type(a) is type(restored)
+    assert a == restored
+    assert a.value == restored.value
+    assert a.name == restored.name
+
+    b = PickleEnum.B
+    pickled = pickle.dumps(b)
+    restored = pickle.loads(pickled)
+    assert type(a) is type(restored)
+    assert b == restored
+    assert b.value == restored.value
+    assert b.name == restored.name