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.

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

Collect models will not create models for not models objects #246

Conversation

macisamuele
Copy link
Collaborator

@macisamuele macisamuele commented Feb 7, 2018

Current model discovery generates models also for not models object specs.
This PR aims to solve this issue.

Assuming that the following is a snippet of the swagger specs

{
    "definitions": {
        "list": {
            "type": "array",
            "items": {"type": "string"},
            "x-model": "list"
        }
    }
    ...
}

Before this PR this generates a list model (assert Swagger.from_url(...).definitions == {'list': mock.ANY})
After this PR assert Swagger.from_url(...).definitions == {}

@coveralls
Copy link

coveralls commented Feb 7, 2018

Coverage Status

Coverage remained the same at 97.942% when pulling 2004e60 on macisamuele:maci-not-object-models-does-not-generate-models into c0825c1 on Yelp:master.

Copy link
Contributor

@sjaensch sjaensch left a comment

Choose a reason for hiding this comment

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

One small nit, lg2m otherwise!



def test_not_model(minimal_swagger_dict, pet_model_spec):
minimal_swagger_dict['definitions']['Pet'] = pet_model_spec
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a docstring explaining why we this is not a model, although it has the x-model marker?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added docstring and updated documentation

@macisamuele macisamuele force-pushed the maci-not-object-models-does-not-generate-models branch from 2555516 to 2004e60 Compare February 9, 2018 11:45
@macisamuele macisamuele merged commit 2ebdd7a into Yelp:master Feb 9, 2018
@macisamuele macisamuele deleted the maci-not-object-models-does-not-generate-models branch February 9, 2018 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants