Use requests instead of httplib in restful_artifact_cache.py

Review Request #294 — Created April 29, 2014 and submitted

johanoskarsson
pants
pants-reviews
benjyw, jsirois, travis
httplib seemed to have poor support for authentication. replaced it with requests that seems to be a blessed lib and has much better support. it also does connection handling for us.
local testing + ci.sh
  • 2
  • 0
  • 0
  • 0
  • 2
Description From Last Updated
Style nit: Use single quotes. BE benjyw
Single quotes throughout. BE benjyw
BE
  1. Looks good, mod the style nits.
  2. Style nit: Use single quotes.
  3. Single quotes throughout.
  4. 
      
JS
  1. small stuff
  2. alphabetize please
  3. This is more appropriate to do in a global spot - though awkward.  For example the ivy bootstrapper uses requests as well - and who knows.  I'd be fine with a TODO since I don't have a nice answer for the potential push-pull of various pants targets using requests in this regard.
  4. It would be good to wrap these branches in a big try/except and translate to CacheError for things like connection errors: http://requests.readthedocs.org/en/latest/user/quickstart/#errors-and-exceptions
  5. 
      
JS
  1. Actually 1 problem - new dep no BUILD edit.  There should be a ~thin target owning this file and it should need a new dep.
  2. 
      
JO
JS
  1. Small stuff.
  2. I need to get the checkstyle tool or similar setup.  The rule is:
    
    [stdlib]
    
    [3rdparty]
    
    [local]
    
    Then within each block alphabetized.
    The urlparse is stdlib - so it goes back up with logging with a newline after it.
    
  3. Safest to just use e and let CacheError super figure out what to do.  The public Exception fields change in python versions.  See: http://legacy.python.org/dev/peps/pep-0352/
    
    Search for 'message'
  4. 
      
JO
Review request changed

Status: Closed (submitted)

Loading...