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

Enable streaming in resource proxy #989

Merged
merged 1 commit into from Jun 26, 2013
Merged

Conversation

domoritz
Copy link
Contributor

@domoritz domoritz commented Jun 9, 2013

Improves performance.

@vitorbaptista
Copy link
Contributor

@domoritz How faster does it get, in your tests? Do you see any reason for not doing it? Also, I get adding stream=True to the requests, but why did you remove decode_unicode=False?

@domoritz
Copy link
Contributor Author

@vitorbaptista The performance difference is sadly to very noticeable because it is not possible to send part of the response before we have downloaded the whole file. However, I should have added streaming=True in the first place because if there is a large file without a content-length header, we would download the whole file. And this wouldn't work. I removed decode_unicode=False because it is the default.

@ghost ghost assigned vitorbaptista Jun 11, 2013
domoritz added a commit that referenced this pull request Jun 12, 2013
@ghost ghost assigned amercader Jun 26, 2013
@amercader amercader merged commit ab6694e into master Jun 26, 2013
@amercader amercader deleted the 989-streaming-res-proxy branch June 26, 2013 13:44
@amercader
Copy link
Member

@vitorbaptista I took this off your hands and merged it, as it was needed for other PRs, thanks!

domoritz added a commit that referenced this pull request Jun 26, 2013
@vitorbaptista
Copy link
Contributor

@amercader Great! Sorry for keeping it for too long 👍

FuhuXia pushed a commit to GSA/ckan that referenced this pull request Mar 5, 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants