Merge pull request #49 from maxmind/greg/fix-travis

horgh · web-flow · commit a52a678964c4 · 2017-12-22T08:21:21.000-08:00
Fix Travis, run yapf from it, and minor code cleanup
diff --git a/.travis-pylint.sh b/.travis-pylint.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+python setup.py install
+pylint geoip2
diff --git a/.travis-yapf.sh b/.travis-yapf.sh
@@ -0,0 +1,14 @@
+#!/bin/bash
+
+diff=$(yapf -rd geoip2 tests)
+
+if [[ $? != 0 ]]; then
+    echo "yapf failed to run."
+    echo "$diff"
+    exit $?
+elif [[ $diff ]]; then
+    echo "$diff"
+    exit 1
+else
+    exit 0
+fi
diff --git a/.travis.yml b/.travis.yml
@@ -31,7 +31,8 @@ install:
   - "if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then pip install ipaddr; fi"
 script:
   - coverage run --source=geoip2 setup.py test
-  - "if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then pylint geoip2; fi"
+  - "if [[ $TRAVIS_PYTHON_VERSION == '3.6' ]]; then ./.travis-pylint.sh; fi"
+  - "if [[ $TRAVIS_PYTHON_VERSION == '3.6' ]]; then ./.travis-yapf.sh; fi"
 after_success:
   - coveralls
 notifications:
diff --git a/geoip2/mixins.py b/geoip2/mixins.py
@@ -9,8 +9,8 @@ class SimpleEquality(object):
     __metaclass__ = ABCMeta
 
     def __eq__(self, other):
-        return (isinstance(other, self.__class__) and
-                self.__dict__ == other.__dict__)
+        return (isinstance(other, self.__class__)
+                and self.__dict__ == other.__dict__)
 
     def __ne__(self, other):
         return not self.__eq__(other)
diff --git a/geoip2/webservice.py b/geoip2/webservice.py
@@ -149,11 +149,10 @@ def _response_for(self, path, model_class, ip_address):
                 'User-Agent': self._user_agent()
             },
             timeout=self._timeout)
-        if response.status_code == 200:
-            body = self._handle_success(response, uri)
-            return model_class(body, locales=self._locales)
-        else:
-            self._handle_error(response, uri)
+        if response.status_code != 200:
+            raise self._exception_for_error(response, uri)
+        body = self._handle_success(response, uri)
+        return model_class(body, locales=self._locales)
 
     def _user_agent(self):
         return 'GeoIP2 Python Client v%s (%s)' % (geoip2.__version__,
@@ -168,57 +167,54 @@ def _handle_success(self, response, uri):
                               'JSON: ' % locals() + ', '.join(ex.args), 200,
                               uri)
 
-    def _handle_error(self, response, uri):
+    def _exception_for_error(self, response, uri):
         status = response.status_code
 
         if 400 <= status < 500:
-            self._handle_4xx_status(response, status, uri)
+            return self._exception_for_4xx_status(response, status, uri)
         elif 500 <= status < 600:
-            self._handle_5xx_status(status, uri)
-        else:
-            self._handle_non_200_status(status, uri)
+            return self._exception_for_5xx_status(status, uri)
+        return self._exception_for_non_200_status(status, uri)
 
-    def _handle_4xx_status(self, response, status, uri):
+    def _exception_for_4xx_status(self, response, status, uri):
         if not response.content:
-            raise HTTPError('Received a %(status)i error for %(uri)s '
-                            'with no body.' % locals(), status, uri)
+            return HTTPError('Received a %(status)i error for %(uri)s '
+                             'with no body.' % locals(), status, uri)
         elif response.headers['Content-Type'].find('json') == -1:
-            raise HTTPError('Received a %i for %s with the following '
-                            'body: %s' % (status, uri, response.content),
-                            status, uri)
+            return HTTPError('Received a %i for %s with the following '
+                             'body: %s' % (status, uri, response.content),
+                             status, uri)
         try:
             body = response.json()
         except ValueError as ex:
-            raise HTTPError(
+            return HTTPError(
                 'Received a %(status)i error for %(uri)s but it did'
                 ' not include the expected JSON body: ' % locals() +
                 ', '.join(ex.args), status, uri)
         else:
             if 'code' in body and 'error' in body:
-                self._handle_web_service_error(
+                return self._exception_for_web_service_error(
                     body.get('error'), body.get('code'), status, uri)
-            else:
-                raise HTTPError(
-                    'Response contains JSON but it does not specify '
-                    'code or error keys', status, uri)
+            return HTTPError('Response contains JSON but it does not specify '
+                             'code or error keys', status, uri)
 
-    def _handle_web_service_error(self, message, code, status, uri):
+    def _exception_for_web_service_error(self, message, code, status, uri):
         if code in ('IP_ADDRESS_NOT_FOUND', 'IP_ADDRESS_RESERVED'):
-            raise AddressNotFoundError(message)
+            return AddressNotFoundError(message)
         elif code in ('AUTHORIZATION_INVALID', 'LICENSE_KEY_REQUIRED',
                       'USER_ID_REQUIRED', 'USER_ID_UNKNOWN'):
-            raise AuthenticationError(message)
+            return AuthenticationError(message)
         elif code in ('INSUFFICIENT_FUNDS', 'OUT_OF_QUERIES'):
-            raise OutOfQueriesError(message)
+            return OutOfQueriesError(message)
         elif code == 'PERMISSION_REQUIRED':
-            raise PermissionRequiredError(message)
+            return PermissionRequiredError(message)
 
-        raise InvalidRequestError(message, code, status, uri)
+        return InvalidRequestError(message, code, status, uri)
 
-    def _handle_5xx_status(self, status, uri):
-        raise HTTPError('Received a server error (%(status)i) for '
-                        '%(uri)s' % locals(), status, uri)
+    def _exception_for_5xx_status(self, status, uri):
+        return HTTPError('Received a server error (%(status)i) for '
+                         '%(uri)s' % locals(), status, uri)
 
-    def _handle_non_200_status(self, status, uri):
-        raise HTTPError('Received a very surprising HTTP status '
-                        '(%(status)i) for %(uri)s' % locals(), status, uri)
+    def _exception_for_non_200_status(self, status, uri):
+        return HTTPError('Received a very surprising HTTP status '
+                         '(%(status)i) for %(uri)s' % locals(), status, uri)
diff --git a/tests/database_test.py b/tests/database_test.py
@@ -87,8 +87,7 @@ def test_asn(self):
         self.assertEqual(record.ip_address, ip_address)
 
         self.assertRegex(
-            str(record),
-            r'geoip2.models.ASN\(.*1\.128\.0\.0.*\)',
+            str(record), r'geoip2.models.ASN\(.*1\.128\.0\.0.*\)',
             'str representation is correct')
 
         self.assertEqual(record, eval(repr(record)), "ASN repr can be eval'd")
@@ -120,8 +119,7 @@ def test_connection_type(self):
             str(record), r'ConnectionType\(\{.*Cable/DSL.*\}\)',
             'ConnectionType str representation is reasonable')
 
-        self.assertEqual(record,
-                         eval(repr(record)),
+        self.assertEqual(record, eval(repr(record)),
                          "ConnectionType repr can be eval'd")
 
         reader.close()
@@ -147,8 +145,8 @@ def test_domain(self):
             str(record), r'Domain\(\{.*maxmind.com.*\}\)',
             'Domain str representation is reasonable')
 
-        self.assertEqual(record,
-                         eval(repr(record)), "Domain repr can be eval'd")
+        self.assertEqual(record, eval(repr(record)),
+                         "Domain repr can be eval'd")
 
         reader.close()
 
diff --git a/tests/models_test.py b/tests/models_test.py
@@ -163,15 +163,14 @@ def test_insights_full(self):
             r'^geoip2.models.Insights\(\{.*geoname_id.*\}, \[.*en.*\]\)',
             'Insights str representation looks reasonable')
 
-        self.assertEqual(model,
-                         eval(repr(model)), "Insights repr can be eval'd")
+        self.assertEqual(model, eval(repr(model)),
+                         "Insights repr can be eval'd")
 
         self.assertRegex(
             str(model.location), r'^geoip2.records.Location\(.*longitude=.*\)',
             'Location str representation is reasonable')
 
-        self.assertEqual(model.location,
-                         eval(repr(model.location)),
+        self.assertEqual(model.location, eval(repr(model.location)),
                          "Location repr can be eval'd")
 
         self.assertTrue(model.traits.is_anonymous)
diff --git a/tests/webservice_test.py b/tests/webservice_test.py
@@ -59,7 +59,9 @@ def test_country_ok(self, mock):
             self.base_uri + 'country/1.2.3.4',
             json=self.country,
             status_code=200,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         country = self.client.country('1.2.3.4')
         self.assertEqual(
             type(country), geoip2.models.Country,
@@ -87,7 +89,9 @@ def test_me(self, mock):
             self.base_uri + 'country/me',
             json=self.country,
             status_code=200,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         implicit_me = self.client.country()
         self.assertEqual(
             type(implicit_me), geoip2.models.Country,
@@ -102,7 +106,9 @@ def test_200_error(self, mock):
         mock.get(
             self.base_uri + 'country/1.1.1.1',
             status_code=200,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(GeoIP2Error,
                                     'could not decode the response as JSON'):
             self.client.country('1.1.1.1')
@@ -119,7 +125,9 @@ def test_no_body_error(self, mock):
             self.base_uri + 'country/' + '1.2.3.7',
             text='',
             status_code=400,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(
                 HTTPError, 'Received a 400 error for .* with no body'):
             self.client.country('1.2.3.7')
@@ -130,7 +138,9 @@ def test_weird_body_error(self, mock):
             self.base_uri + 'country/' + '1.2.3.8',
             text='{"wierd": 42}',
             status_code=400,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(HTTPError,
                                     'Response contains JSON but it does not '
                                     'specify code or error keys'):
@@ -142,7 +152,9 @@ def test_bad_body_error(self, mock):
             self.base_uri + 'country/' + '1.2.3.9',
             text='bad body',
             status_code=400,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(
                 HTTPError, 'it did not include the expected JSON body'):
             self.client.country('1.2.3.9')
@@ -159,7 +171,9 @@ def test_300_error(self, mock):
         mock.get(
             self.base_uri + 'country/' + '1.2.3.11',
             status_code=300,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(HTTPError,
                                     'Received a very surprising HTTP status '
                                     '\(300\) for'):
@@ -214,7 +228,9 @@ def _test_error(self, mock, status, error_code, error_class):
             self.base_uri + 'country/1.2.3.18',
             json=body,
             status_code=status,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(error_class, msg):
             self.client.country('1.2.3.18')
 
@@ -227,7 +243,9 @@ def test_unknown_error(self, mock):
             self.base_uri + 'country/' + ip,
             json=body,
             status_code=400,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         with self.assertRaisesRegex(InvalidRequestError, msg):
             self.client.country(ip)
 
@@ -237,7 +255,9 @@ def test_request(self, mock):
             self.base_uri + 'country/' + '1.2.3.4',
             json=self.country,
             status_code=200,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         self.client.country('1.2.3.4')
         request = mock.request_history[-1]
 
@@ -256,7 +276,9 @@ def test_city_ok(self, mock):
             self.base_uri + 'city/' + '1.2.3.4',
             json=self.country,
             status_code=200,
-            headers={'Content-Type': self._content_type('city')})
+            headers={
+                'Content-Type': self._content_type('city')
+            })
         city = self.client.city('1.2.3.4')
         self.assertEqual(
             type(city), geoip2.models.City, 'return value of client.city')
@@ -267,7 +289,9 @@ def test_insights_ok(self, mock):
             self.base_uri + 'insights/1.2.3.4',
             json=self.country,
             status_code=200,
-            headers={'Content-Type': self._content_type('country')})
+            headers={
+                'Content-Type': self._content_type('country')
+            })
         insights = self.client.insights('1.2.3.4')
         self.assertEqual(
             type(insights), geoip2.models.Insights,

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+#!/bin/sh`
	`2`	`+python setup.py install`
	`3`	`+pylint geoip2`