Skip to content
This repository has been archived by the owner on Oct 19, 2022. It is now read-only.

Add metadata for apispec compatibility #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Birne94
Copy link

@Birne94 Birne94 commented Aug 15, 2018

fixes #24

This PR adds the key enum to the field's metadata which allows apispec (and possibly other tools) to extract the valid enum values.

@@ -71,6 +71,11 @@ def __init__(

super(EnumField, self).__init__(*args, **kwargs)

self.metadata['enum'] = [
e.value if self.by_value else e.name
Copy link
Owner

@justanr justanr Aug 16, 2018

Choose a reason for hiding this comment

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

by_value needs to be deprecated in favor of the load_by and dump_by attributes so this isn't a reliable path forward. The other issue is that you can load by either name/value and dump by name/value independently. I'd rather not deprecate this behavior as it's actually a nice way of working around not great third party APIs (e.g. load from their bad API into our nicer one, and then dump out our nicer version).

Something that could be done is creating a dictionary here of "Accepts" and "Returns" (or something similar) and loading both keys with a list of what they actually do. I imagine that 99% of the time they'll be the same but since this functionality exists, it should be supported fully.

Copy link
Author

Choose a reason for hiding this comment

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

As far as I can tell, the openapi spec does not allow differentiating between using a schema for loading and for dumping data.

Would it be a reasonable approach to set the enum metadata to either the values or the names if both load_by and dump_by are equal and use both values and names in other cases?

For the test this would result in

class Test(Enum):
    one = 1
    two = 2
    three = 3
load_by dump_by metadata['enum']
name name one,two,three
value value 1,2,3
name value one,two,three,1,2,3
value name one,two,three,1,2,3

Copy link
Owner

Choose a reason for hiding this comment

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

Seems that'd be confusing to end users. As for differentiating between the sent model and returned model, is that a limit of apispec or the marshmallow extension? Because that's a fairly common use case in apis.

Choose a reason for hiding this comment

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

I think @Birne94 's proposal adds a lot of value for the

99% of the time they'll be the same

and is also my personal preference for the cases where they're not as well (If automatic tooling can't get the exact set of allowable values, at least it narrows it down to a superset which is no more than 2x the set of allowable values for a person who cares about one side of the api--either the ingested schema or the output schema).

@justanr if you disagree, would not setting it for load_by != dump_by cases be preferable? Then it does no harm if it can't get it exactly right, and is helpful in the cases where it definitely can.

Choose a reason for hiding this comment

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

Has there been any activity on a decision regarding this MR? We've just started using this library, and ran into this exact use case and could use a fix.

Copy link

@h h Mar 12, 2020

Choose a reason for hiding this comment

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

As for differentiating between the sent model and returned model, is that a limit of apispec or the marshmallow extension? Because that's a fairly common use case in apis.

This is actually a limitation of the OpenAPI specification itself. I'd suggest either raising an error, or falling back on the previous behavior when load_by/dump_by are unequal.

@quaterneon quaterneon mentioned this pull request Jan 30, 2019
@vpassapera
Copy link

It would be nice if we could get this merged. I am having troubles getting EnumField to work correctly currently with apispec.

TypeError: 'MyEnum' object is not subscriptable

@h
Copy link

h commented Mar 12, 2020

Could you please merge this PR? We're also running into this same issue of being unable to export Enum values using apispec and marshmallow_dataclass. Would be fantastic to have this!

h added a commit to h/marshmallow_enum that referenced this pull request Mar 12, 2020
@h
Copy link

h commented Mar 12, 2020

I added a fork (based on #25) that fixes this issue (while removing the load_by/dump_by ambiguity) at https://github.com/h/marshmallow_enum

This issue hasn't been merged since 2018 -- so if anyone else needs support for APISpec with marshmallow, simply run:

$ pip install -e git+https://github.com/h/marshmallow_enum#egg=marshmallow-enum

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Apispec / OpenApi
6 participants