reviewboard/scmtools/hg.py (Diff revision 1)
Should we just be replacing https with http? If https has been specified, the user might have an expectation of encryption which is not being provided.
Switch away from 'requests' and standardize SCMTool HTTP usage.
Review Request #2901 — Created Feb. 19, 2012 and submitted
Switch away from 'requests' and standardize SCMTool HTTP usage. The 'requests' module, while nice on the outside, does not meet our needs today. It doesn't support Python 2.4 or 2.5 (which RB 1.6.x does support), and it's unclear what support will be going forward. In reality, the main reason we're using it in 1.6.x at all is because we needed to pre-populate the Authorization header, which requests will do automatically. However, this isn't anything special. We can just add that header ourselves and keep urllib2, which is fine because our needs there are very basic. In order to prevent code duplication, I've created a SCMClient class that Git's and Mercurial's clients inherit from. It provides at the moment one utility function, get_file_http, which wraps the urllib2 logic. If we switch back at some point to requests or another module, we need only update this function. We probably won't need to, though. In the end, both classes are simplified, we don't need this dependency, we keep our Python support, and pre-populated authorizations work.
Tested this with a couple different Git and Mercurial repos, with and without HTTP Basic Auth. In particular, I made sure this still worked with CodebaseHQ's Git repository. I also confirmed with Wireshark that the first request had the Authorization header.
Can you make this more verbose? If it doesn't include the source URL, just looking at the "error" log will …
Should we just be replacing https with http? If https has been specified, the user might have an expectation of …
Well, I'm sad that it turned out to be a dud. I <3 the API :( This change looks pretty good. Just one comment.
reviewboard/scmtools/core.py (Diff revision 1)
Can you make this more verbose? If it doesn't include the source URL, just looking at the "error" log will be useless.