Repository: mapbox/mapbox-sdk-py
Pull request #47 (2015-10-26 19:16:15)

handle_http_error
`handle_http_error` in base Service class to encapsulate details of how we deal with unsuccessful HTTP status codes.

Allows you to write service methods like so

```
        resp = self.session.get(uri, params=params)
        self.handle_http_error(resp)
        return resp
```

I didn't want to deal with calling the base class'  `__init__` everywhere so it just takes a `raise_for_status` boolean at the moment.

Also added the idea of custom messages so that we could raise `HTTPError` with a specific message under certain conditions (see `Uploader._get_credentials` where, because the method hides network, we need to raise but with an appropriate message about token scope)

applied to upload and geocoder


Patch:
diff --git a/mapbox/services/base.py b/mapbox/services/base.py
index cd972fa..b06eed3 100644
--- a/mapbox/services/base.py
+++ b/mapbox/services/base.py
@@ -12,3 +12,13 @@ class Service:
         session = requests.Session()
         session.params.update(access_token=access_token)
         return session
+
+    def handle_http_error(self, response, custom_messages=None,
+                          raise_for_status=False):
+        if not custom_messages:
+            custom_messages = {}
+        if response.status_code in custom_messages.keys():
+            raise requests.exceptions.HTTPError(
+                custom_messages[response.status_code])
+        if raise_for_status:
+            response.raise_for_status()
diff --git a/mapbox/services/geocoding.py b/mapbox/services/geocoding.py
index 16dca78..8180e55 100644
--- a/mapbox/services/geocoding.py
+++ b/mapbox/services/geocoding.py
@@ -41,7 +41,9 @@ class Geocoder(Service):
             params.update(self._validate_place_types(types))
         if lon is not None and lat is not None:
             params.update(proximity='{0},{1}'.format(round(float(lon), self.precision.get('proximity', 3)), round(float(lat), self.precision.get('proximity', 3))))
-        return self.session.get(uri, params=params)
+        resp = self.session.get(uri, params=params)
+        self.handle_http_error(resp)
+        return resp
 
     def reverse(self, lon=None, lat=None, types=None):
         """Returns a Requests response object that contains a GeoJSON
@@ -56,7 +58,9 @@ class Geocoder(Service):
         params = {}
         if types:
             params.update(self._validate_place_types(types))
-        return self.session.get(uri, params=params)
+        resp = self.session.get(uri, params=params)
+        self.handle_http_error(resp)
+        return resp
 
     @property
     def place_types(self):
diff --git a/mapbox/services/upload.py b/mapbox/services/upload.py
index 880cab5..b1f85fe 100644
--- a/mapbox/services/upload.py
+++ b/mapbox/services/upload.py
@@ -34,7 +34,11 @@ class Uploader(Service):
         """
         uri = URITemplate('%s/{username}/credentials' % self.baseuri).expand(
             username=self.username)
-        return self.session.get(uri)
+        resp = self.session.get(uri)
+        self.handle_http_error(
+            resp,
+            custom_messages={401: "Token does not have upload scope"})
+        return resp
 
     def stage(self, filepath, creds=None):
         """Stages the user's file on S3
@@ -84,7 +88,9 @@ class Uploader(Service):
         uri = URITemplate('%s/{username}' % self.baseuri).expand(
             username=self.username)
 
-        return self.session.post(uri, json=msg)
+        resp = self.session.post(uri, json=msg)
+        self.handle_http_error(resp)
+        return resp
 
     def list(self):
         """List of all uploads
@@ -94,7 +100,9 @@ class Uploader(Service):
         """
         uri = URITemplate('%s/{username}' % self.baseuri).expand(
             username=self.username)
-        return self.session.get(uri)
+        resp = self.session.get(uri)
+        self.handle_http_error(resp)
+        return resp
 
     def delete(self, upload):
         """Delete the specified upload
@@ -106,7 +114,9 @@ class Uploader(Service):
 
         uri = URITemplate('%s/{username}/{upload_id}' % self.baseuri).expand(
             username=self.username, upload_id=upload_id)
-        return self.session.delete(uri)
+        resp = self.session.delete(uri)
+        self.handle_http_error(resp)
+        return resp
 
     def status(self, upload):
         """Check status of upload
@@ -121,7 +131,9 @@ class Uploader(Service):
 
         uri = URITemplate('%s/{username}/{upload_id}' % self.baseuri).expand(
             username=self.username, upload_id=upload_id)
-        return self.session.get(uri)
+        resp = self.session.get(uri)
+        self.handle_http_error(resp)
+        return resp
 
     def upload(self, filepath, tileset):
         """High level function to upload a local file to mapbox tileset


Test patch:
diff --git a/tests/test_base.py b/tests/test_base.py
new file mode 100644
index 0000000..41718f6
--- /dev/null
+++ b/tests/test_base.py
@@ -0,0 +1,51 @@
+import mapbox
+import responses
+import pytest
+import requests
+
+
+def test_service_session():
+    """Get a session using a token"""
+    session = mapbox.Service().get_session('pk.test')
+    assert session.params.get('access_token') == 'pk.test'
+
+
+def test_service_session_env():
+    """Get a session using the env's token"""
+    session = mapbox.Service().get_session(
+        env={'MapboxAccessToken': 'pk.test_env'})
+    assert session.params.get('access_token') == 'pk.test_env'
+
+
+def test_service_session_os_environ(monkeypatch):
+    """Get a session using os.environ's token"""
+    monkeypatch.setenv('MapboxAccessToken', 'pk.test_os_environ')
+    session = mapbox.Service().get_session()
+    assert session.params.get('access_token') == 'pk.test_os_environ'
+    monkeypatch.undo()
+
+
+def test_service_session_os_environ_caps(monkeypatch):
+    """Get a session using os.environ's token"""
+    monkeypatch.setenv('MAPBOX_ACCESS_TOKEN', 'pk.test_os_environ')
+    session = mapbox.Service().get_session()
+    assert session.params.get('access_token') == 'pk.test_os_environ'
+    monkeypatch.undo()
+
+
+@responses.activate
+def test_custom_messages():
+    fakeurl = 'https://example.com'
+    responses.add(responses.GET, fakeurl, status=401)
+    service = mapbox.Service()
+    response = service.get_session().get(fakeurl)
+
+    assert service.handle_http_error(response) is None
+
+    with pytest.raises(requests.exceptions.HTTPError) as exc:
+        assert service.handle_http_error(response, custom_messages={401: "error"})
+        assert exc.value.message == 'error'
+
+    with pytest.raises(requests.exceptions.HTTPError) as exc:
+        assert service.handle_http_error(response, raise_for_status=True)
+        assert "401" in exc.value.message
diff --git a/tests/test_geocoder.py b/tests/test_geocoder.py
index cfe95ef..6424904 100644
--- a/tests/test_geocoder.py
+++ b/tests/test_geocoder.py
@@ -5,35 +5,6 @@ import responses
 import mapbox
 
 
-def test_service_session():
-    """Get a session using a token"""
-    session = mapbox.Service().get_session('pk.test')
-    assert session.params.get('access_token') == 'pk.test'
-
-
-def test_service_session_env():
-    """Get a session using the env's token"""
-    session = mapbox.Service().get_session(
-        env={'MapboxAccessToken': 'pk.test_env'})
-    assert session.params.get('access_token') == 'pk.test_env'
-
-
-def test_service_session_os_environ(monkeypatch):
-    """Get a session using os.environ's token"""
-    monkeypatch.setenv('MapboxAccessToken', 'pk.test_os_environ')
-    session = mapbox.Service().get_session()
-    assert session.params.get('access_token') == 'pk.test_os_environ'
-    monkeypatch.undo()
-
-
-def test_service_session_os_environ_caps(monkeypatch):
-    """Get a session using os.environ's token"""
-    monkeypatch.setenv('MAPBOX_ACCESS_TOKEN', 'pk.test_os_environ')
-    session = mapbox.Service().get_session()
-    assert session.params.get('access_token') == 'pk.test_os_environ'
-    monkeypatch.undo()
-
-
 def test_geocoder_default_name():
     """Default name is set"""
     geocoder = mapbox.Geocoder()
@@ -216,4 +187,4 @@ def test_geocoder_reverse_rounding():
     match = re.search(r'\/([\-\d\.\,]+)\.json', response.url)
     assert match is not None
     for coord in re.split(r'(%2C|,)', match.group(1)):
-        assert _check_coordinate_precision(coord, 5)
\ No newline at end of file
+        assert _check_coordinate_precision(coord, 5)