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
feat: Adding support for streamed responses #6
Conversation
Issue: #5 This adds support for a streamed response and is based on the implementation I saw https://github.com/lostisland/faraday-net_http/blob/9534fd19bd4898f28361ea60dbc3867edadc15ad/lib/faraday/adapter/net_http.rb#L106-L120
http_response | ||
else | ||
http.request(env[:url], create_request(env)) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is based on https://github.com/lostisland/faraday-net_http/blob/9534fd19bd4898f28361ea60dbc3867edadc15ad/lib/faraday/adapter/net_http.rb#L106-L120
Though I'm happy to hear if you think there is a better way :)
My manual tests look good. I tested this feature against a get of a largish (12 MB) file. The size and md5sum were the same for all of these tests run against this feature branch:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much @MikeRogers0 for porting this feature from the net_http
adapter and @wconrad for testing with large files as well 🙌
LGTM 🎉 !
You guys are truly awesome. Thanks @iMacTia and @MikeRogers0 ! |
lostisland/faraday-net_http_persistent#6 implemented streaming response support and it was release in 1.2.0 (see https://github.com/lostisland/faraday-net_http_persistent/releases/tag/v1.2.0)
lostisland/faraday-net_http_persistent#6 implemented streaming response support and it was release in 1.2.0 (see https://github.com/lostisland/faraday-net_http_persistent/releases/tag/v1.2.0)
Closes: #5
This adds support for a streamed response and is based on the
implementation I saw https://github.com/lostisland/faraday-net_http/blob/9534fd19bd4898f28361ea60dbc3867edadc15ad/lib/faraday/adapter/net_http.rb#L106-L120
The tests are all passing (Yay), I also ran the steps to reproduce provided by @wconrad (Thank you so much, this helped a bunch) locally & had the result (I also tried a few other links, though I didn't check any massive files):