Merge pull request #289 from scieloorg/codex/configurable-objectstore-timeout

Make objectstore timeout configurable

Author: gitnnolabs

README.md

@@ -77,10 +77,11 @@ https://docs.mongodb.com/master/core/transactions/.
 Configurações avançadas:
 
 
-variável de ambiente      | valor padrão
---------------------------|-------------
-KERNEL_LIB_MAX_RETRIES    | 4
-KERNEL_LIB_BACKOFF_FACTOR | 1.2
+variável de ambiente           | valor padrão
+-------------------------------|-------------
+KERNEL_LIB_MAX_RETRIES         | 4
+KERNEL_LIB_BACKOFF_FACTOR      | 1.2
+KERNEL_LIB_OBJECTSTORE_TIMEOUT | 2
 
 ### Executando via código-fonte e Pip:
 

documentstore/domain.py

@@ -41,6 +41,7 @@
 
 MAX_RETRIES = int(os.environ.get("KERNEL_LIB_MAX_RETRIES", "4"))
 BACKOFF_FACTOR = float(os.environ.get("KERNEL_LIB_BACKOFF_FACTOR", "1.2"))
+OBJECTSTORE_TIMEOUT = float(os.environ.get("KERNEL_LIB_OBJECTSTORE_TIMEOUT", "2"))
 OBJECTSTORE_RESPONSE_TIME_SECONDS = Summary(
     "kernel_objectstore_response_time_seconds",
     "Elapsed time between the request for an XML and the response",
@@ -55,6 +56,14 @@ def utcnow():
     return str(datetime.utcnow().isoformat() + "Z")
 
 
+def objectstore_timeout(timeout=None):
+    if timeout is None:
+        return float(
+            os.environ.get("KERNEL_LIB_OBJECTSTORE_TIMEOUT", OBJECTSTORE_TIMEOUT)
+        )
+    return timeout
+
+
 class DocumentManifest:
     """Namespace para funções que manipulam o manifesto do documento.
     """
@@ -231,7 +240,8 @@ def wrapper(*args, **kwargs):
 @retry_gracefully()
 @OBJECTSTORE_REQUEST_FAILURES_TOTAL.count_exceptions()
 @OBJECTSTORE_RESPONSE_TIME_SECONDS.time()
-def fetch_data(url: str, timeout: float = 2) -> bytes:
+def fetch_data(url: str, timeout: float = None) -> bytes:
+    timeout = objectstore_timeout(timeout)
     try:
         response = requests.get(url, timeout=timeout)
     except (requests.exceptions.ConnectionError, requests.exceptions.Timeout) as exc:
@@ -257,7 +267,7 @@ def fetch_data(url: str, timeout: float = 2) -> bytes:
 
 
 def assets_from_remote_xml(
-    url: str, timeout: float = 2, parser=DEFAULT_XMLPARSER
+    url: str, timeout: float = None, parser=DEFAULT_XMLPARSER
 ) -> list:
     data = fetch_data(url, timeout)
     xml = etree.parse(BytesIO(data), parser)
@@ -350,7 +360,11 @@ def id(self):
         return self.manifest.get("id", "")
 
     def new_version(
-        self, data_url, assets_getter=assets_from_remote_xml, timeout=2, ensure_unique_name=False
+        self,
+        data_url,
+        assets_getter=assets_from_remote_xml,
+        timeout=None,
+        ensure_unique_name=False,
     ) -> None:
         """Adiciona `data_url` como uma nova versão do documento.
 
@@ -371,7 +385,7 @@ def new_version(
                     "could not add version: the version is equal to the latest one"
                 )
 
-        _, data_assets = assets_getter(data_url, timeout=timeout)
+        _, data_assets = assets_getter(data_url, timeout=objectstore_timeout(timeout))
         data_assets_keys = [asset_key for asset_key, _ in data_assets]
         assets = self._link_assets(data_assets_keys)
         self.manifest = DocumentManifest.add_version(
@@ -500,7 +514,7 @@ def data(
         version_index=-1,
         version_at=None,
         assets_getter=assets_from_remote_xml,
-        timeout=2,
+        timeout=None,
     ) -> bytes:
         """Retorna o conteúdo do XML, codificado em UTF-8, já com as
         referências aos ativos digitais correspondendo às da versão solicitada.
@@ -526,7 +540,9 @@ def data(
             raise exceptions.DeletedVersion(
                 "cannot get data: the document was deleted")
 
-        xml_tree, data_assets = assets_getter(version["data"], timeout=timeout)
+        xml_tree, data_assets = assets_getter(
+            version["data"], timeout=objectstore_timeout(timeout)
+        )
 
         version_assets = version["assets"]
         for asset_key, target_node in data_assets:

tests/test_domain.py

@@ -2026,6 +2026,43 @@ def test_sleep_increases_exponentially(self):
         retry_gracefully._sleep.assert_has_calls(calls)
 
 
+class ObjectstoreTimeoutTests(unittest.TestCase):
+    def test_default_timeout_value(self):
+        with mock.patch.dict(domain.os.environ, {}, clear=True):
+            self.assertEqual(domain.objectstore_timeout(), 2)
+
+    def test_timeout_value_from_environment(self):
+        with mock.patch.dict(
+            domain.os.environ, {"KERNEL_LIB_OBJECTSTORE_TIMEOUT": "15.5"}
+        ):
+            self.assertEqual(domain.objectstore_timeout(), 15.5)
+
+    def test_explicit_timeout_value_has_priority(self):
+        with mock.patch.dict(
+            domain.os.environ, {"KERNEL_LIB_OBJECTSTORE_TIMEOUT": "15.5"}
+        ):
+            self.assertEqual(domain.objectstore_timeout(3), 3)
+
+    def test_fetch_data_uses_configured_timeout(self):
+        response = mock.Mock()
+        response.content = b"<article/>"
+        response.raise_for_status.return_value = None
+
+        with mock.patch.dict(
+            domain.os.environ, {"KERNEL_LIB_OBJECTSTORE_TIMEOUT": "15.5"}
+        ):
+            with mock.patch(
+                "documentstore.domain.requests.get", return_value=response
+            ) as request_get:
+                self.assertEqual(
+                    domain.fetch_data("https://example.org/doc.xml"), b"<article/>"
+                )
+
+        request_get.assert_called_once_with(
+            "https://example.org/doc.xml", timeout=15.5
+        )
+
+
 class MetadataWithStylesForArticleWithTransTitlesTests(unittest.TestCase):
 
     def setUp(self):