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

Excon streaming #1026

Merged
merged 4 commits into from
Sep 22, 2019
Merged

Excon streaming #1026

merged 4 commits into from
Sep 22, 2019

Conversation

technoweenie
Copy link
Member

This implements streaming response bodies in Excon. Confirmed with technoweenie/faraday-live#8, if you test this branch:

$ FARADAY_GEM_REF=excon_streaming docker-compose build tests
$ TEST_PROTO=https TEST_ADAPTER=excon TEST_METHOD=get docker-compose run tests

Note: Excon claims to yield the chunk, remaining bytes, and total bytes. In practice, it only seems to do this if the response came in as 1 chunk. Or maybe, it skips this on chunked responses that don't give the content size up front. Either way, Faraday has to maintain its own total byte count.

irb(main):003:0> Excon.get('http://geemus.com', response_block: lambda { |b,r,t| puts "RECV #{b.size}, rem #{r.to_i}/#{t.to_i} #{Digest::SHA1.hexdigest(b)}" });nil
RECV 3722, rem 0/3722 f5878b59cc37c7cc306a78cac34362ab52c72e46
=> nil
irb(main):004:0> Excon.get('https://jigsaw.w3.org/HTTP/ChunkedScript', response_block: lambda { |b,r,t| puts "RECV #{b.size}, rem #{r.to_i}/#{t.to_i} #{Digest::SHA1.hexdigest(b)}" });nil
RECV 9329, rem 0/0 61218bfa9a16320a916775562eece77bf4b82786
RECV 16304, rem 0/0 a547ac23bcd520b7c4fada5ac58aab3e781c7d07
RECV 2244, rem 0/0 25931bac8db4b27b866fd0d0ead8302a2d335c94
RECV 1428, rem 0/0 476afc2e63062d56f5e92fd5b167b06fd1c3e664
RECV 8560, rem 0/0 67dc7ba1c6123f67fde2dce96cd5e41cba317d6f
RECV 12836, rem 0/0 6bad2a8664449eae0bcf3bc6f0ae1b66c9584dc3
RECV 5712, rem 0/0 c011d596e925bf265f7660c12820499b3843b7dd
RECV 9980, rem 0/0 e92f6efb3d1e4e376580781cf04ea944bf5734d9
RECV 5807, rem 0/0 08e1d0def8b8a73542469fc95214049b9ffe6cec

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

This reads great and makes sense - prepare the req_opts before using them.

@technoweenie technoweenie merged commit 872a249 into master Sep 22, 2019
@technoweenie technoweenie deleted the excon_streaming branch September 22, 2019 15:43
technoweenie added a commit to technoweenie/faraday-live that referenced this pull request Sep 22, 2019
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.

2 participants