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

Fix infinite recursion in swagger.py for circular models #656

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Drarok
Copy link

@Drarok Drarok commented Jun 18, 2019

With thanks to @edwardfung123 from Dec 2016 for identifying this fix, fixes #190.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 96.822% when pulling fd8358a on Drarok:fix/infinite-recursion-bug into e911078 on noirbizarre:master.

@coveralls
Copy link

coveralls commented Jun 18, 2019

Coverage Status

Coverage decreased (-0.04%) to 96.867% when pulling 9ab99ea on Drarok:fix/infinite-recursion-bug into a7e363a on noirbizarre:master.

@StephS
Copy link

StephS commented Aug 22, 2019

any update on this? i ran into the same bug. This PR fixes it.

@j5awry
Copy link
Collaborator

j5awry commented Aug 22, 2019

Thank you for contributing. Please add relevant tests to cover the issue.

@gvoropaev
Copy link

gvoropaev commented Aug 30, 2019

I'm waiting for @Drarok to merge new test in his branch. It will fix the coverage issue.

@Drarok
Copy link
Author

Drarok commented Aug 30, 2019

@buggyspace it’s a two line change, and I’m not even sure how I would begin to test for the failure case.

@gvoropaev
Copy link

gvoropaev commented Aug 30, 2019

@Drarok I've already done it

Drarok#1

@Drarok
Copy link
Author

Drarok commented Aug 30, 2019

@buggyspace oh, amazing! Thank you! For some reason I didn't get a notification about that.

I've brought your commit over and rebased on current master, so we should be good to go?

@gvoropaev
Copy link

@Drarok yes, thank you.

@j5awry can you start the review, please?

@idchlife
Copy link
Contributor

idchlife commented Sep 8, 2019

Any news on merging this, guys? I think circular model reference for api doc is pretty usual thing occuring from time to time in any project.

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.

Swagger UI crashes when circular model defined
6 participants