Merge pull request #171 from metaodi/add-timeout

Add timeout parameter

CHANGELOG.md

@@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project follows [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
+### Added
+- Add a new `timeout` parameter to `OsmApi` which allows to set a timeout in seconds (default is 30s) for the API requests (see issue #170, thanks [Mateusz Konieczny](https://github.com/matkoniecz))
+
 ### Changed
 - Only include `discussion` key in result of `ChangesetGet` if `include_discussion=True` (see issue #163, thanks [Mateusz Konieczny](https://github.com/matkoniecz))
 - Update OAuth example in README using [cli-oauth2](https://github.com/Zverik/cli-oauth2) (see PR #169, thanks [Ilya Zverev](https://github.com/Zverik)

examples/timeout.py

@@ -0,0 +1,33 @@
+# install cli-oauth2 for requests:  pip install cli-oauth2
+from oauthcli import OpenStreetMapDevAuth
+import osmapi
+from dotenv import load_dotenv, find_dotenv
+import os
+
+load_dotenv(find_dotenv())
+
+# load secrets for OAuth
+client_id = os.getenv("OSM_OAUTH_CLIENT_ID")
+client_secret = os.getenv("OSM_OAUTH_CLIENT_SECRET")
+
+auth = OpenStreetMapDevAuth(
+    client_id, client_secret, ["write_api", "write_notes"]
+).auth_code()
+
+
+# Use a normal timeout (30s is the default value)
+normal_timeout_api = osmapi.OsmApi(
+    api="https://api06.dev.openstreetmap.org", session=auth.session, timeout=30
+)
+changeset_id = normal_timeout_api.ChangesetCreate({"comment": "My first test"})
+print(f"Create new changeset {changeset_id}")
+
+# Deliberatly using a very small timeout to show what happens when a timeout occurs
+low_timeout_api = osmapi.OsmApi(
+    api="https://api06.dev.openstreetmap.org", session=auth.session, timeout=0.00001
+)
+try:
+    changeset_id = low_timeout_api.ChangesetCreate({"comment": "My first test"})
+    print(f"Create new changeset {changeset_id}")
+except osmapi.errors.TimeoutApiError as e:
+    print(f"Timeout error occured: {str(e)}")

osmapi/OsmApi.py

@@ -61,6 +61,7 @@ def __init__(
         changesetautosize=500,
         changesetautomulti=1,
         session=None,
+        timeout=30,
     ):
         """
         Initialized the OsmApi object.
@@ -93,6 +94,14 @@ def __init__(
         upload (default: 500) and `changesetautomulti` defines how many
         uploads should be made before closing a changeset and opening a new
         one (default: 1).
+
+        The `session` parameter can be used to provide a custom requests
+        http session object (requests.Session). This might be useful for
+        OAuth authentication, custom adapters, hooks etc.
+
+        Finally the `timeout` parameter is used by the http session to
+        throw an expcetion if the the timeout (in seconds) has passed without
+        an answer from the server.
         """
 
         # Get username
@@ -144,16 +153,24 @@ def __init__(
 
         # Http connection
         self.http_session = session
+        self._timeout = timeout
         auth = None
         if self._username and self._password:
             auth = (self._username, self._password)
         self._session = http.OsmApiSession(
-            self._api, self._created_by, auth=auth, session=self.http_session
+            self._api,
+            self._created_by,
+            auth=auth,
+            session=self.http_session,
+            timeout=self._timeout,
         )
 
     def __enter__(self):
         self._session = http.OsmApiSession(
-            self._api, self._created_by, session=self.http_session
+            self._api,
+            self._created_by,
+            session=self.http_session,
+            timeout=self._timeout,
         )
         return self
 

osmapi/errors.py

@@ -144,3 +144,9 @@ class PreconditionFailedApiError(ApiError):
     - When a relation has elements that do not exist or are not visible
     - When a node/way/relation is still used in a way/relation
     """
+
+
+class TimeoutApiError(ApiError):
+    """
+    Error if the http request ran into a timeout
+    """

osmapi/http.py

@@ -14,9 +14,10 @@ class OsmApiSession:
     MAX_RETRY_LIMIT = 5
     """Maximum retries if a call to the remote API fails (default: 5)"""
 
-    def __init__(self, base_url, created_by, auth=None, session=None):
+    def __init__(self, base_url, created_by, auth=None, session=None, timeout=30):
         self._api = base_url
         self._created_by = created_by
+        self._timeout = timeout
 
         try:
             self._auth = auth
@@ -68,7 +69,17 @@ def _http_request(self, method, path, auth, send, return_value=True):  # noqa
         if auth and not self._auth:
             raise errors.UsernamePasswordMissingError("Username/Password missing")
 
-        response = self._session.request(method, path, data=send)
+        try:
+            response = self._session.request(
+                method, path, data=send, timeout=self._timeout
+            )
+        except requests.exceptions.Timeout:
+            raise errors.TimeoutApiError(
+                0, f"Request timed out (timeout={self._timeout})", ""
+            )
+        except requests.exceptions.RequestException as e:
+            raise errors.ApiError(0, str(e), "")
+
         if response.status_code != 200:
             payload = response.content.strip()
             if response.status_code == 401:
@@ -106,9 +117,10 @@ def _http(self, cmd, path, auth, send, return_value=True):  # noqa
                         self._sleep()
                     self._session = self._get_http_session()
                 else:
+                    logger.exception("ApiError Exception occured")
                     raise
             except Exception as e:
-                logger.error(e)
+                logger.exception("General exception occured")
                 if i == self.MAX_RETRY_LIMIT:
                     if isinstance(e, errors.OsmApiError):
                         raise

tests/changeset_test.py

@@ -3,6 +3,7 @@
 import datetime
 import pytest
 from responses import GET, PUT, POST
+import requests
 
 
 def xmltosorteddict(xml):
@@ -62,6 +63,18 @@ def test_ChangesetGet(api, add_response):
     assert result == test_changeset
 
 
+def test_ChangesetGet_with_timeout(api, add_response):
+    # Setup mock
+    add_response(GET, "/changeset/123", body=requests.exceptions.Timeout())
+
+    # Call
+    with pytest.raises(osmapi.TimeoutApiError) as execinfo:
+        api.ChangesetGet(123)
+    assert (
+        str(execinfo.value) == "Request failed: 0 - Request timed out (timeout=30) - "
+    )
+
+
 def test_ChangesetUpdate(auth_api, add_response):
     # Setup mock
     resp = add_response(PUT, "/changeset/create", filename="test_ChangesetCreate.xml")

tests/helper_test.py

@@ -59,7 +59,7 @@ def test_close_context_manager(self):
     def test_http_request_get(self):
         response = self.api._session._http_request("GET", "/api/0.6/test", False, None)
         self.mock_session.request.assert_called_with(
-            "GET", self.api_base + "/api/0.6/test", data=None
+            "GET", self.api_base + "/api/0.6/test", data=None, timeout=30
         )
         self.assertEqual(response, "test response")
         self.assertEqual(self.mock_session.request.call_count, 1)
@@ -70,7 +70,7 @@ def test_http_request_put(self):
             "PUT", "/api/0.6/testput", False, data
         )
         self.mock_session.request.assert_called_with(
-            "PUT", self.api_base + "/api/0.6/testput", data="data"
+            "PUT", self.api_base + "/api/0.6/testput", data="data", timeout=30
         )
         self.assertEqual(response, "test response")
 
@@ -80,7 +80,7 @@ def test_http_request_delete(self):
             "PUT", "/api/0.6/testdelete", False, data
         )
         self.mock_session.request.assert_called_with(
-            "PUT", self.api_base + "/api/0.6/testdelete", data="delete data"
+            "PUT", self.api_base + "/api/0.6/testdelete", data="delete data", timeout=30
         )
         self.assertEqual(response, "test response")
 
@@ -89,7 +89,7 @@ def test_http_request_auth(self):
             "PUT", "/api/0.6/testauth", True, None
         )
         self.mock_session.request.assert_called_with(
-            "PUT", self.api_base + "/api/0.6/testauth", data=None
+            "PUT", self.api_base + "/api/0.6/testauth", data=None, timeout=30
         )
         self.assertEqual(self.mock_session.auth, ("testuser", "testpassword"))
         self.assertEqual(response, "test response")