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

Return raw response #225

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

Conversation

gstarnberger
Copy link
Contributor

Adds a new config option "return_raw_response" that returns the raw decoded response (after json/msgpack decoding, skipping the call to unmarshal_schema_object).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 97.973% when pulling 12effce on gstarnberger:return_raw_response into 6f595cb on Yelp:master.

@@ -115,6 +115,8 @@ def has_content(response_spec):
content_value = umsgpack.loads(response.raw_bytes)
if op.swagger_spec.config['validate_responses']:
validate_schema_object(op.swagger_spec, content_spec, content_value)
if op.swagger_spec.config['return_raw_response']:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This config seems more related to "HTTP" handling and not swagger. I would suggest adding this into bravado implementation as request option so you could enable this behavior on per request base.

@gstarnberger
Copy link
Contributor Author

I looked at request_options but I didn't find a way to implement the same functionality (without larger code changes). Essentially what I'd like to do is:

  1. Add a new option that skips the call to unmarshal_schema_object for latency-sensitive use-cases.

  2. Still be able to enable options such as validate_responses in dev/stage environments to verify that requests/responses match the spec.

All of this logic (response validation, response decoding, ...) is called in bravado_core.response.unmarshal_response so if I'd move the new code to a higher level I'd have to duplicate some of the unmarshal_response logic somewhere else (e.g., response validation, content-type detection, ...). For return_raw_response I followed the approach used for the use_models config option which is similar as it also determines how the response is returned.

How would an implementation that uses request options look like? Would I have to duplicate the deserialization logic (json, msgpack) and the response validation logic somewhere else?

@macisamuele
Copy link
Collaborator

@gstarnberger fair point ... During hackathon @sjaensch started to work a bit on async io bravado and we were discussing if it could make sense to move into bravado the logic for handling requests, so that could be already a sort of solution for you.

For your use case, I was thinking to add a request option (similar to also_return_response) into bravado such that once it is enabled we're completely skipping the bravado-core interaction -> no validation, no unmarshaling and you still have access to the raw response (so bytes in case of msgpack).

Let me sync better with @sjaensch tomorrow morning, he could eventually have better ideas about your case 😄

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