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.

Sign up for GitHub

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

Jump to bottom

Improve enum compatibility #1153

Merged
merged 11 commits into from
Jul 13, 2020
Merged

Improve enum compatibility #1153

merged 11 commits into from
Jul 13, 2020

Conversation

jkimbo
Copy link
Member

@jkimbo jkimbo commented Mar 14, 2020 •
edited
Loading

This PR improves enum compatibility by allowing resolvers to return enums as well as their values and names. It also changes the input values for mutations to be the enum rather than the enum value.

Example:

from enum import Enum as PyEnum

class Color(PyEnum):
    RED = 1
    GREEN = 2
    BLUE = 3

GColor = Enum.from_enum(Color)

class Query(ObjectType):
    color = GColor(required=True)
    color_by_value = GColor(required=True)

    def resolve_color(_, info):
        return Color.RED

    def resolve_color_by_value(_, info):
        return Color.RED.value

schema = Schema(query=Query)

results = schema.execute(
    """query {
      color
      colorByValue
    }"""
)
assert not results.errors

assert results.data["color"] == "RED"
assert results.data["colorByValue"] == "RED"

Since this change is backwards incompatible by changing the way mutations with enum inputs work I've also added a compatibility middleware (enum_value_convertor_middleware) that can be added so that mutations behave as they did. This should give people the ability to migrate their applications after upgrading to v3. We decided against adding a middleware because it was too complicated and probably wouldn't have worked properly anyway. Decided to go with documenting the change instead.

TODO

@jkimbo jkimbo requested a review from ekampf March 14, 2020 16:57
This was referenced Mar 14, 2020
Closed
Closed
@stale
Copy link

stale bot commented Jun 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 12, 2020
@jkimbo jkimbo force-pushed the improve-enum-compatablilty branch from d80937d to 31fb2da Compare June 25, 2020 09:49
@jkimbo jkimbo marked this pull request as ready for review June 25, 2020 10:51
ekampf
ekampf approved these changes Jun 28, 2020
View reviewed changes
Copy link
Contributor

@ekampf ekampf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! 🎊

@syrusakbary
Copy link
Member

I think this PR needs further review and deep analysis. I'm not sure the middleware approach is the optimal one to solve the compatibility with Graphene 2

@jkimbo jkimbo requested a review from ekampf July 9, 2020 15:18
@jkimbo jkimbo force-pushed the improve-enum-compatablilty branch from 26229d6 to 1d3525e Compare July 9, 2020 15:37
@patrick91 patrick91 mentioned this pull request Jul 10, 2020
Open
@syrusakbary
Copy link
Member

Looks great. Merging

@syrusakbary syrusakbary merged commit 81fff0f into master Jul 13, 2020
@fabienheureux fabienheureux mentioned this pull request Aug 7, 2020
Merged
@jkimbo jkimbo deleted the improve-enum-compatablilty branch August 26, 2020 15:08
@jkimbo jkimbo mentioned this pull request Oct 21, 2020
Closed
majgaard added a commit to majgaard/graphene that referenced this pull request Jun 24, 2021
@majgaard
Revert "Improve enum compatibility (graphql-python#1153)"
e7c4768 
This reverts commit 81fff0f.
@fescobar
Copy link

Hi @jkimbo
I would like to know when it will be released version 3 of graphene-django?
https://pypi.org/project/graphene-django/#history
There is no updates for a long time ago.
Thank you in advance.

karisal-anders added a commit to City-of-Helsinki/kukkuu that referenced this pull request Jun 20, 2024
@karisal-anders
feat: add graphene_enum_values function & test it
66044b0 
might be useful because Graphene 3 has changed from using enum values to
enums, for example:

```python
class TestEnum(graphene.Enum):
  FIRST = 'first'
  SECOND = 'second'
```

would've been serialized previously using the enum values, i.e.
'first' and 'second'. But with Graphene 3 they are serialized as
'TestEnum.FIRST' and 'TestEnum.SECOND'. This breaks functionality as
parts of the codebase are expecting the enum values as per Graphene 2.

Related https://github.com/graphql-python/graphene issues & PRs:
 - "Improve enum compatibility" PR:
   - graphql-python/graphene#1153
 - "graphene3: enum doesn't resolve to value" issue:
   - graphql-python/graphene#1277
 - "I would like my enum input values to be the enum instance instead
   of the enum values" issue:
   - graphql-python/graphene#1151

refs KK-1108
@karisal-anders karisal-anders mentioned this pull request Jun 20, 2024
Merged
karisal-anders added a commit to City-of-Helsinki/kukkuu that referenced this pull request Jul 8, 2024
@karisal-anders
feat: add graphene_enum_values function & test it
09a3d02 
might be useful because Graphene 3 has changed from using enum values to
enums, for example:

```python
class TestEnum(graphene.Enum):
  FIRST = 'first'
  SECOND = 'second'
```

would've been serialized previously using the enum values, i.e.
'first' and 'second'. But with Graphene 3 they are serialized as
'TestEnum.FIRST' and 'TestEnum.SECOND'. This breaks functionality as
parts of the codebase are expecting the enum values as per Graphene 2.

Related https://github.com/graphql-python/graphene issues & PRs:
 - "Improve enum compatibility" PR:
   - graphql-python/graphene#1153
 - "graphene3: enum doesn't resolve to value" issue:
   - graphql-python/graphene#1277
 - "I would like my enum input values to be the enum instance instead
   of the enum values" issue:
   - graphql-python/graphene#1151

refs KK-1108
karisal-anders added a commit to City-of-Helsinki/kukkuu that referenced this pull request Jul 11, 2024
@karisal-anders
feat: add graphene_enum_values function & test it
6229ae4 
might be useful because Graphene 3 has changed from using enum values to
enums, for example:

```python
class TestEnum(graphene.Enum):
  FIRST = 'first'
  SECOND = 'second'
```

would've been serialized previously using the enum values, i.e.
'first' and 'second'. But with Graphene 3 they are serialized as
'TestEnum.FIRST' and 'TestEnum.SECOND'. This breaks functionality as
parts of the codebase are expecting the enum values as per Graphene 2.

Related https://github.com/graphql-python/graphene issues & PRs:
 - "Improve enum compatibility" PR:
   - graphql-python/graphene#1153
 - "graphene3: enum doesn't resolve to value" issue:
   - graphql-python/graphene#1277
 - "I would like my enum input values to be the enum instance instead
   of the enum values" issue:
   - graphql-python/graphene#1151

refs KK-1108
karisal-anders added a commit to City-of-Helsinki/kukkuu that referenced this pull request Jul 15, 2024
@karisal-anders
feat: add graphene_enum_values function & test it
a4633fe 
might be useful because Graphene 3 has changed from using enum values to
enums, for example:

```python
class TestEnum(graphene.Enum):
  FIRST = 'first'
  SECOND = 'second'
```

would've been serialized previously using the enum values, i.e.
'first' and 'second'. But with Graphene 3 they are serialized as
'TestEnum.FIRST' and 'TestEnum.SECOND'. This breaks functionality as
parts of the codebase are expecting the enum values as per Graphene 2.

Related https://github.com/graphql-python/graphene issues & PRs:
 - "Improve enum compatibility" PR:
   - graphql-python/graphene#1153
 - "graphene3: enum doesn't resolve to value" issue:
   - graphql-python/graphene#1277
 - "I would like my enum input values to be the enum instance instead
   of the enum values" issue:
   - graphql-python/graphene#1151

refs KK-1108
karisal-anders added a commit to City-of-Helsinki/palvelutarjotin that referenced this pull request Oct 9, 2024
@karisal-anders
feat: upgrade all dependencies, PostgreSQL 10->13, use ruff
cda2b8e 
Done:
 - Upgrade all dependencies
 - Upgrade postgresql from v10 to v13 (v13 is used in production)
 - Use ruff instead of black/isort/flake8, remove noqa's, reformat
 - Use pyproject.toml instead of setup.cfg

Background for @map_enums_to_values_in_kwargs decorator:

Graphene 3 changed from using enum values to enums, for example:
```python
class TestEnum(graphene.Enum):
  FIRST = 'first'
  SECOND = 'second'
```
would've been serialized previously using the enum values, i.e.
'first' and 'second'. But with Graphene 3 they are serialized as
'TestEnum.FIRST' and 'TestEnum.SECOND'. This broke functionality as
parts of the codebase were expecting the enum values as per Graphene 2.

Forced backwards compatibility by forcefully mapping enums to their
values.

Related https://github.com/graphql-python/graphene issues & PRs:
 - "Improve enum compatibility" PR:
   - graphql-python/graphene#1153
 - "graphene3: enum doesn't resolve to value" issue:
   - graphql-python/graphene#1277
 - "I would like my enum input values to be the enum instance instead
   of the enum values" issue:
   - graphql-python/graphene#1151

See https://github.com/graphql-python/graphene/wiki/v3-release-notes
for Graphene v3's breaking changes,

refs PT-1730
@karisal-anders karisal-anders mentioned this pull request Oct 9, 2024
Merged
karisal-anders added a commit to City-of-Helsinki/palvelutarjotin that referenced this pull request Oct 11, 2024
@karisal-anders
feat: upgrade all dependencies, PostgreSQL 10->13, use ruff
bcbf88b 
Done:
 - Upgrade all dependencies
 - Upgrade postgresql from v10 to v13 (v13 is used in production)
 - Use ruff instead of black/isort/flake8, remove noqa's, reformat
 - Use pyproject.toml instead of setup.cfg

Background for @map_enums_to_values_in_kwargs decorator:

Graphene 3 changed from using enum values to enums, for example:
```python
class TestEnum(graphene.Enum):
  FIRST = 'first'
  SECOND = 'second'
```
would've been serialized previously using the enum values, i.e.
'first' and 'second'. But with Graphene 3 they are serialized as
'TestEnum.FIRST' and 'TestEnum.SECOND'. This broke functionality as
parts of the codebase were expecting the enum values as per Graphene 2.

Forced backwards compatibility by forcefully mapping enums to their
values.

Related https://github.com/graphql-python/graphene issues & PRs:
 - "Improve enum compatibility" PR:
   - graphql-python/graphene#1153
 - "graphene3: enum doesn't resolve to value" issue:
   - graphql-python/graphene#1277
 - "I would like my enum input values to be the enum instance instead
   of the enum values" issue:
   - graphql-python/graphene#1151

See https://github.com/graphql-python/graphene/wiki/v3-release-notes
for Graphene v3's breaking changes,

refs PT-1730
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekampf ekampf ekampf approved these changes

@syrusakbary syrusakbary syrusakbary approved these changes

@mvanlonden mvanlonden Awaiting requested review from mvanlonden

@dan98765 dan98765 Awaiting requested review from dan98765

Assignees
No one assigned
Labels
✨ enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkimbo @syrusakbary @fescobar @ekampf