Change FormatParser.parse_http to follow a redirect #190
Conversation
fabioperrella
force-pushed
the
follow-redirect
branch
from
94ca9b2 to
584ed63
Compare
fabioperrella
force-pushed
the
follow-redirect
branch
from
2f34e41 to
88facdf
Compare
fabioperrella
force-pushed
the
follow-redirect
branch
from
88facdf to
db19c62
Compare
Co-authored-by: Andrei Horak <[email protected]>
Co-authored-by: Andrei Horak <[email protected]>
@lorenzograndi4 I saw here that the default value is 3. It seems a reasonable value for me. Do you think we should use another value? |
|
3 sounds good ! |
| @@ -7,7 +7,9 @@ | |||
| rio = described_class.new('https://images.invalid/img.jpg') | |||
|
|
|||
| fake_resp = double(headers: {'Content-Range' => '10-109/2577'}, status: 206, body: 'This is the response') | |||
| expect_any_instance_of(Faraday::Connection).to receive(:get).with('https://images.invalid/img.jpg', nil, range: 'bytes=10-109').and_return(fake_resp) | |||
| faraday_conn = instance_double(Faraday::Connection, get: fake_resp) | |||
There was a problem hiding this comment.
You could extract the double in a let and the allow in a before, to DRY things up a bit 😊
There was a problem hiding this comment.
I tried but I think I can't do it.
Faraday is loaded only after calling FormatParser::RemoteIO.new (here):
def initialize(uri)
require 'faraday'I think it was done this way to avoid loading the gem when it's not necessary, for example, if you parse only local files.
So, when I tried to reference the Faraday class in the before block, I got the error uninitialized constant Faraday.
I think that it's worth to require the gem before just to change the test.
Is it ok for you?
There was a problem hiding this comment.
Ah, right...
Looking at it better, the subject could also be DRYed out, and that would also require the gem. It is more work than this PR covers, so I'm also fine with moving on with how it is at the moment.
closes #189
This PR adds support to follow a redirect when downloading a file with
FormatParser.parse_httpI was inspired by this article which explains how to follow a redirect using Faraday