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

Support for Swagger 2.0 requests #101

Merged
merged 24 commits into from Apr 23, 2015

Conversation

analogue
Copy link
Contributor

  • Swagger 1.2 is not in the code path at all.
  • tween20.py created to isolate Swagger 2.0 support
  • Manually tested using example_happyhour and works wonderfully
  • using requirements.txt for deps until packaged on pypi

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.56%) to 95.16% when pulling ce4caf5 on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-4.59%) to 95.13% when pulling 3e99874 on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.96% when pulling 3e99874 on analogue:swagger_2.0_requests into fe3b877 on striglia:master.

1 similar comment
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.96% when pulling 3e99874 on analogue:swagger_2.0_requests into fe3b877 on striglia:master.

@analogue
Copy link
Contributor Author

@dnephin Think this is ready to go on the request side. I have another branch which is for the response side of things. Let me know what you think.

'validate_path',
'exclude_paths',
'exclude_routes',
]
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks identical to the one in tween.py, (except for spec vs schema name). Could we use the one in tween.py for now, and rename it to schema to spec at some point later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack

@dnephin
Copy link
Contributor

dnephin commented Apr 23, 2015

I think this looks good. With the proposed changes it could be just three functions (get_op_for_request, swaggerize_request, and swaggerize_response) with some very minor changes to the existing code to parametrize it.

@landscape-bot
Copy link

Code Health
Repository health decreased by 2% when pulling 0b5337d on analogue:swagger_2.0_requests into fe3b877 on striglia:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.79%) to 90.93% when pulling ac18ef5 on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-8.79%) to 90.93% when pulling ac18ef5 on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

@landscape-bot
Copy link

Code Health
Repository health decreased by 2% when pulling ac18ef5 on analogue:swagger_2.0_requests into fe3b877 on striglia:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.79%) to 90.93% when pulling 96879c1 on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-8.79%) to 90.93% when pulling 96879c1 on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

@landscape-bot
Copy link

Code Health
Repository health decreased by 2% when pulling 96879c1 on analogue:swagger_2.0_requests into fe3b877 on striglia:master.

validator_map)
validation_context=validation_context,
validator_map=op_or_validators_map,
op=op_or_validators_map)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is passing the same argument in twice, why not just use positional args and pass it in once?

@landscape-bot
Copy link

Code Health
Repository health decreased by 2% when pulling dfe22aa on analogue:swagger_2.0_requests into fe3b877 on striglia:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.81%) to 90.91% when pulling dfe22aa on analogue:swagger_2.0_requests into fe3b877 on striglia:swagger2.0.

validate_response(response, validator_map.response)
swagger_handler.handle_response(
response,
validator=getattr(op_or_validators_map, 'response', None))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could remove this getattr by adjusting the validate_response() to take the op_or_validators_map instead, but that seems fine to wait until the response branch.

@dnephin
Copy link
Contributor

dnephin commented Apr 23, 2015

lgtm

dnephin added a commit that referenced this pull request Apr 23, 2015
@dnephin dnephin merged commit 0611457 into Yelp:swagger2.0 Apr 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants