forgo urllib2 in favour of requests (bug 858729)

lib/crypto/receipt.py

@@ -1,12 +1,11 @@
 import json
-import urllib2
 
 from django.conf import settings
 from django_statsd.clients import statsd
 
 import commonware.log
-
 import jwt
+import requests
 
 
 log = commonware.log.getLogger('z.crypto')
@@ -34,30 +33,28 @@ def sign(receipt):
     log.info('Receipt contents: %s' % receipt_json)
     headers = {'Content-Type': 'application/json'}
     data = receipt if isinstance(receipt, basestring) else receipt_json
-    request = urllib2.Request(destination, data, headers)
 
     try:
         with statsd.timer('services.sign.receipt'):
-            response = urllib2.urlopen(request, timeout=timeout)
-    except urllib2.HTTPError, error:
-        # Will occur when a 3xx or greater code is returned
-        msg = error.read().strip()
-        log.error('Posting to receipt signing failed: %s, %s'
-                  % (error.code, msg))
-        raise SigningError('Posting to receipt signing failed: %s, %s'
-                           % (error.code, msg))
-    except:
+            req = requests.get(destination, data=data, headers=headers,
+                               timeout=timeout)
+    except requests.Timeout:
+        statsd.incr('services.sign.receipt.timeout')
+        log.error('Posting to receipt signing timed out')
+        raise SigningError('Posting to receipt signing timed out')
+    except requests.RequestException:
         # Will occur when some other error occurs.
+        statsd.incr('services.sign.receipt.error')
         log.error('Posting to receipt signing failed', exc_info=True)
-        raise SigningError('Posting receipt signing failed')
+        raise SigningError('Posting to receipt signing failed')
 
-    if response.getcode() != 200:
-        log.error('Posting to signing failed: %s'
-                  % (response.getcode()))
+    if req.status_code != 200:
+        statsd.incr('services.sign.receipt.error')
+        log.error('Posting to signing failed: %s' % req.status_code)
         raise SigningError('Posting to signing failed: %s'
-                           % (response.getcode()))
+                           % req.status_code)
 
-    return json.loads(response.read())['receipt']
+    return json.loads(req.content)['receipt']
 
 
 def decode(receipt):

lib/crypto/tests.py

@@ -10,6 +10,7 @@
 import jwt
 import mock
 from nose.tools import eq_, raises
+from requests import Timeout
 
 import amo.tests
 from lib.crypto import packaged
@@ -37,38 +38,41 @@ def mock_sign(version_id, reviewer=False):
     return path
 
 
-@mock.patch('lib.crypto.receipt.urllib2.urlopen')
+@mock.patch('lib.crypto.receipt.requests.get')
 @mock.patch.object(settings, 'SIGNING_SERVER', 'http://localhost')
 class TestReceipt(amo.tests.TestCase):
 
-    def test_called(self, urlopen):
-        urlopen.return_value = self.get_response(200)
+    def test_called(self, get):
+        get.return_value = self.get_response(200)
         sign('my-receipt')
-        eq_(urlopen.call_args[0][0].data, 'my-receipt')
+        eq_(get.call_args[1]['data'], 'my-receipt')
 
-    def test_some_unicode(self, urlopen):
-        urlopen.return_value = self.get_response(200)
+    def test_some_unicode(self, get):
+        get.return_value = self.get_response(200)
         sign({'name': u'Вагиф Сәмәдоғлу'})
 
     def get_response(self, code):
-        response = mock.Mock()
-        response.getcode = mock.Mock()
-        response.getcode.return_value = code
-        response.read.return_value = json.dumps({'receipt': ''})
-        return response
+        return mock.Mock(status_code=code,
+                         content=json.dumps({'receipt': ''}))
+
+    def test_good(self, req):
+        req.return_value = self.get_response(200)
+        sign('x')
 
     @raises(SigningError)
-    def test_error(self, urlopen):
-        urlopen.return_value = self.get_response(403)
+    def test_timeout(self, req):
+        req.side_effect = Timeout
+        req.return_value = self.get_response(200)
         sign('x')
 
-    def test_good(self, urlopen):
-        urlopen.return_value = self.get_response(200)
+    @raises(SigningError)
+    def test_error(self, req):
+        req.return_value = self.get_response(403)
         sign('x')
 
     @raises(SigningError)
-    def test_other(self, urlopen):
-        urlopen.return_value = self.get_response(206)
+    def test_other(self, req):
+        req.return_value = self.get_response(206)
         sign('x')
 
 

mkt/developers/tests/test_tasks.py

@@ -395,9 +395,11 @@ def test_response_too_large(self):
         self.check_validation('Your manifest must be less than %s bytes.' %
                               max_webapp_size)
 
+    @mock.patch('mkt.developers.tasks.validator', lambda uid, **kw: None)
     def test_http_error(self):
-        self.requests_mock.side_effect = urllib2.HTTPError(
-            'url', 404, 'Not Found', [], None)
+        with self.patch_requests() as ur:
+            ur.status_code = 404
+
         tasks.fetch_manifest('url', self.upload.pk)
         self.check_validation(
             'No manifest was found at that URL. Check the address and try '