Add Range header to HTTP::perform_head_request #425
Conversation
spec/spec_helper.rb
Outdated
| @@ -45,7 +45,7 @@ def copy_fixture_to_pod(name, pod) | |||
| VCR.configure do |c| | |||
| c.cassette_library_dir = ROOT + 'spec/fixtures/vcr_cassettes' | |||
| c.hook_into :webmock | |||
| c.ignore_hosts 'codeclimate.com' | |||
| c.ignore_hosts 'codeclimate.com', 'some-url' | |||
There was a problem hiding this comment.
I dont think this host should be ignored?
There was a problem hiding this comment.
You're right.
I was thinking that if you forget to use WebMock to mock an endpoint, then VCR shouldn't try to handle it since it's not a legitimate URL to begin with and will never be recorded as a cassette. However, it's better that VCR complains about it than for Net::HTTP to try to connect to it.
|
@segiddins Do you think it's preferable to have the original fallback GET request as a third fallback case, or just get rid of it entirely? |
kylefleming
force-pushed
the
perform_head_request_range_header
branch
from
b29847b
to
fe29dd2
Compare
I think its a preferable fallback |
|
Shall I squash it? |
|
@kylefleming sure |
Uses a GET request with a 1 byte Range header if the initial HEAD request fails. Falls back to a GET request without a Range header if that also fails.
kylefleming
force-pushed
the
perform_head_request_range_header
branch
from
fe29dd2
to
5c700e2
Compare
|
danke! |
|
Do we have an estimate on when 1.5 will be released? Still waiting on this fix so I can push the latest OpenCV pod (CocoaPods/trunk.cocoapods.org#236). |
|
@kylefleming not yet, but you can point to Core master using a gemfile to use it now: https://guides.cocoapods.org/using/unreleased-features.html |
|
@segiddins Thanks for the suggestion, but I actually need https://github.com/CocoaPods/trunk.cocoapods.org to be updated with this fix in order to push the pod. |
|
@segiddins Do you think it will be awhile before it's released? I'm inquiring so that I know whether I should be looking into workarounds or not. |
|
I don't know how long it will be |
Uses a GET request with a 1 byte Range header if the initial HEAD request fails. Falls back to a GET request without a Range header if that also fails.
Closes #420
Discussion
I've added the GET request with a Range header as an additional fallback and kept the original non-Range GET request as the final fallback. This is because I wasn't sure if we should expect that a server might respond to a normal GET request but return an error if the GET request includes a Range header.
My intuition would say that almost all servers that don't respond to a Range header would just ignore it and proceed as if it didn't exist. In this case, we wouldn't need the third non-Range fallback. Do we think this is a case that we would want to support? Having all 3 is the more conservative route, but adds additional latency in returning a result if the server doesn't have the content at all.
I think ideally we'd remove the third non-Range fallback, but I wanted to submit this PR with it in, in case it is decided that it's necessary.