If you're feeling ambitious, we could also replace this with ``` @classmethod @memoized_method def _get_ping_time(url_info] ``` On the Pinger class, instead of manually implementing the memoization. Up to you though.
Maybe it's worth going the distance and using requests instead of urllib2? We already use requests in several places, and urllib2 only in one other, obscure place.
The two lines collapse down to:
requests.head(url, timeout=timeout)which is a little neater, and is also explicit about the fact that it uses HEAD, instead of hiding that behind the somewhat opaque info() call.
Why do we need this? Does urllib2's conn.info() send a GET request, not a HEAD request?
If so, I'm even more in favor of using requests instead of urllib2.
Make pinger.py work with both HTTP and HTTPS.
Review Request #3904 — Created May 17, 2016 and submitted
|benjyw, patricklaw, stuhood, zundel|
Our remote artifact cache serves over HTTPS, which the RESTful
artifact cache ostensibly supports, but the Pinger was previously
checking what URLs are reachable using only HTTP.
This change switches pinger to use requests instead of httplib,
which gives us HTTPS support "for-free".
Added test to pinger.py. All tests in pinger.py pass locally.
Jenkins went green: http://jenkins.pantsbuild.org/job/pantsbuild/job/pants/branch/PR-3463/5/