Yelp / bravado

Bravado is a python client library for Swagger 2.0 services
Other
604 stars 117 forks source link

Slow handling response of large binary files (application/octet format) #394

Open rubyshamir opened 6 years ago

rubyshamir commented 6 years ago

Hello and thanks for this great software!

We have a swagger definition containing a "GET" command returning a large file (format: "application/octet-stream").

The current implementation of response() eventually calls "unmarshal_response_inner" in "http_future.py". This method does not have a specific handling for binary data and encodes the data to string.

On large files, like in our case, this is a very time consuming process (2-3 seconds per 0.5 MB). We have locally altered the code to return the binary data in case of "application/octet-stream" format and the time was reduced to ~0.1 seconds per 0.5 MB.

The altered function is as follows:

def unmarshal_response_inner(response, op):    
    ...
    # Code we added ##
    if content_type.startswith('application/octet'):
        return response.raw_bytes

    # TODO: Non-json response contents
    return response.text

We will be glad to learn if there is another workaround without changing the code.

Thanks in advance,

Ruby

gp-greg commented 5 years ago

We also encountered this scenario recently and implemented a similar workaround. Would you accept a PR with a change like Ruby has suggested? In our case we are working with Excel files so our content type is application/vnd.openxmlformats-officedocument.spreadsheetml.sheet.

macisamuele commented 5 years ago

@greenphire-greg PRs are always welcome 😉

I'm still unsure about what should be the more appropriate return value in case of a response is not json or msg-pack.

gp-greg commented 5 years ago

I was hesitant to suggest changing the default return value too. I made a PR in the bravado-core repo, which if accepted I believe will need to be re-implemented here as well? (The distinction between bravado and bravado-core is not totally clear to me.)

https://github.com/Yelp/bravado-core/pull/317

advance512 commented 3 years ago

Would be nicer to also do this for image/, audio/, video/* responses.