Merge formatting features with caching

Author: olliesilvester

Dockerfile

@@ -27,8 +27,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
 COPY --from=build /venv/ /venv/
 COPY tests/test_data/beamline_parameters.txt tests/test_data/beamline_parameters.txt
 ENV PATH=/venv/bin:$PATH
-ARG BEAMLINE="dev"
-ENV BEAMLINE=${BEAMLINE}
 
 # change this entrypoint if it is not the same as the repo
 CMD daq-config-server

pyproject.toml

@@ -17,7 +17,7 @@ readme = "README.md"
 requires-python = ">=3.11"
 
 [project.optional-dependencies]
-server = ["fastapi", "uvicorn", "redis", "hiredis"]
+server = ["fastapi", "uvicorn", "redis", "hiredis", "cachetools"]
 dev = [
     "copier",
     "httpx",
@@ -31,6 +31,9 @@ dev = [
     "ruff",
     "tox-direct",
     "types-mock",
+    "cachetools",
+    "fastapi",
+    "uvicorn",
 ]
 
 [project.scripts]

src/daq_config_server/__main__.py

@@ -14,12 +14,10 @@ def check_server_dependencies():
         import uvicorn  # noqa
         from fastapi import FastAPI  # noqa
 
-        server_dependencies_exist = True
+        return True
 
     except ImportError:
-        server_dependencies_exist = False
-
-    return server_dependencies_exist
+        return False
 
 
 def main():

src/daq_config_server/client.py

@@ -1,16 +1,16 @@
+import operator
 from enum import StrEnum
 from logging import Logger, getLogger
-from typing import Any, TypeVar
+from typing import Any
 
 import requests
+from cachetools import TTLCache, cachedmethod
+from requests import Response
 
 from daq_config_server.app import ValidAcceptHeaders
 
 from .constants import ENDPOINTS
 
-T = TypeVar("T")
-BlParamDType = str | int | float | bool
-
 
 class RequestedResponseFormats(StrEnum):
     DICT = ValidAcceptHeaders.JSON  # Convert to dict using json.loads()
@@ -19,26 +19,77 @@ class RequestedResponseFormats(StrEnum):
 
 
 class ConfigServer:
-    def __init__(self, url: str, log: Logger | None = None) -> None:
+    def __init__(
+        self,
+        url: str,
+        log: Logger | None = None,
+        cache_size: int = 10,
+        cache_lifetime_s: int = 3600,
+    ) -> None:
+        """
+        Initialize the ConfigServer client.
+
+        Args:
+            url: Base URL of the config server.
+            log: Optional logger instance.
+            cache_size: Size of the cache (maximum number of items can be stored).
+            cache_lifetime_s: Lifetime of the cache (in seconds).
+        """
         self._url = url.rstrip("/")
         self._log = log if log else getLogger("daq_config_server.client")
+        self._cache = TTLCache(maxsize=cache_size, ttl=cache_lifetime_s)
+
+    @cachedmethod(cache=operator.attrgetter("_cache"))
+    def _cached_get(
+        self,
+        endpoint: str,
+        accept_header: str,
+        item: str,
+    ) -> Response:
+        """
+        Get data from the config server and cache it.
+
+        Args:
+            endpoint: API endpoint.
+            item: item identifier - the filepath.
+
+        Returns:
+            The response data.
+        """
+
+        try:
+            r = requests.get(
+                self._url + endpoint + (f"/{item}"), headers={"Accept": accept_header}
+            )
+            r.raise_for_status()
+            self._log.debug(f"Cache set for {endpoint}/{item}.")
+            return r
+        except requests.exceptions.HTTPError as e:
+            self._log.error(f"HTTP error: {e}")
+            raise
 
     def _get(
         self,
         endpoint: str,
-        headers: dict,
-        item: str | None = None,
+        accept_header: str,
+        item: str,
+        reset_cached_result=False,
     ):
-        r = requests.get(
-            self._url + endpoint + (f"/{item}" if item else ""), headers=headers
-        )
+        """
+        Get data from the config server with cache management and use
+        the content-type response header to format the return value.
+        If data parsing fails, return the response contents in bytes
+        """
+        if (endpoint, accept_header, item) in self._cache and reset_cached_result:
+            del self._cache[(endpoint, accept_header, item)]
+        r = self._cached_get(endpoint, accept_header, item)
 
         content_type = r.headers["content-type"].split(";")[0].strip()
 
-        if content_type != headers["Accept"]:
+        if content_type != accept_header:
             self._log.warning(
                 f"Server failed to parse the file as requested. Requested \
-                {headers['Accept']} but response came as content-type {content_type}"
+                {accept_header} but response came as content-type {content_type}"
             )
 
         try:
@@ -64,17 +115,25 @@ def get_file_contents(
         requested_response_format: RequestedResponseFormats = (
             RequestedResponseFormats.DECODED_STRING
         ),
+        reset_cached_result: bool = False,
     ) -> Any:
         """
-        Get an file contents from the config server in the format specified.
-
-        If data parsing fails, the return type will be bytes
+        Get contents of a file from the config server in the format specified.
+        If data parsing fails, the return type will be bytes. Optionally try to use
+        cached result.
 
         Args:
             file_path: Path to the file.
             requested_response_format: Specify how to parse the response.
+            reset_cached_result: Whether to reset cache for specific request.
         Returns:
-            The file contents, in the type specified.
+            The file contents, in the format specified.
         """
-        headers = {"Accept": requested_response_format}
-        return self._get(ENDPOINTS.CONFIG, headers, file_path)
+
+        accept_header = requested_response_format
+        return self._get(
+            ENDPOINTS.CONFIG,
+            accept_header,
+            file_path,
+            reset_cached_result=reset_cached_result,
+        )

tests/system_tests/test_client.py

@@ -9,9 +9,9 @@
 
 """For now, these tests require locally hosting the config server
 
-While in the python environment, run
+While in the python environment, run from the terminal:
 
-python -m daq_config_server --dev
+daq-config-server
 
 before running the tests
 """

tests/unit_tests/test_client.py

@@ -1,23 +1,45 @@
 from unittest.mock import MagicMock, patch
 
+import pytest
+import requests
 from fastapi import status
 from httpx import Response
+from requests import RequestException
 
 from daq_config_server.app import ValidAcceptHeaders
 from daq_config_server.client import ConfigServer, RequestedResponseFormats
 from daq_config_server.constants import ENDPOINTS
 
 
+def make_test_response(
+    content: str,
+    status_code: int = 200,
+    raise_exc: type[RequestException] | None = None,
+    json_value: str | None = None,
+    content_type=ValidAcceptHeaders.PLAIN_TEXT,
+):
+    r = Response(
+        json=json_value,
+        status_code=status_code,
+        headers={"content-type": content_type},
+        content=content,
+    )
+    r.raise_for_status = MagicMock()
+
+    if raise_exc:
+        r.raise_for_status.side_effect = raise_exc
+    else:
+        r.raise_for_status.return_value = None
+    return r
+
+
 @patch("daq_config_server.client.requests.get")
 def test_get_file_contents_default_header(mock_request: MagicMock):
-    content_type = ValidAcceptHeaders.PLAIN_TEXT
     mock_request.return_value = Response(
         status_code=status.HTTP_200_OK,
         content="test",
-        headers={
-            "content-type": content_type,
-        },
     )
+    mock_request.return_value = make_test_response("test")
     file_path = "test"
     url = "url"
     server = ConfigServer(url)
@@ -34,13 +56,8 @@ def test_get_file_contents_warns_and_gives_bytes_on_invalid_json(
 ):
     content_type = ValidAcceptHeaders.JSON
     bad_json = "bad_dict}"
-    mock_request.return_value = Response(
-        status_code=status.HTTP_200_OK,
-        json="test",
-        headers={
-            "content-type": content_type,
-        },
-        content=bad_json,
+    mock_request.return_value = make_test_response(
+        bad_json, content_type=content_type, json_value=bad_json
     )
     file_path = "test"
     url = "url"
@@ -59,28 +76,31 @@ def test_get_file_contents_warns_and_gives_bytes_on_invalid_json(
 
 
 @patch("daq_config_server.client.requests.get")
-def test_logger_warning_if_content_type_doesnt_match_requested_type(
+def test_read_unformatted_file_reading_reset_cached_result_true_without_cache(
     mock_request: MagicMock,
 ):
-    headers = {"Accept": RequestedResponseFormats.DICT}
-    content_type = ValidAcceptHeaders.PLAIN_TEXT
-    text = "text"
-    mock_request.return_value = Response(
-        status_code=status.HTTP_200_OK,
-        headers={
-            "content-type": content_type,
-        },
-        content=text,
-    )
+    """Test reset_cached_result=False and reset_cached_result=True."""
+    mock_request.side_effect = [
+        make_test_response("1st_read"),
+        make_test_response("2nd_read"),
+        make_test_response("3rd_read"),
+    ]
     file_path = "test"
     url = "url"
     server = ConfigServer(url)
-    server._log.warning = MagicMock()
-    server.get_file_contents(
-        file_path, requested_response_format=RequestedResponseFormats.DICT
-    )
+    assert server.get_file_contents(file_path, reset_cached_result=True) == "1st_read"
+    assert server.get_file_contents(file_path, reset_cached_result=True) == "2nd_read"
+    assert server.get_file_contents(file_path, reset_cached_result=False) == "2nd_read"
+
 
-    server._log.warning.assert_called_once_with(
-        f"Server failed to parse the file as requested. Requested \
-                {headers['Accept']} but response came as content-type {content_type}"
+@patch("daq_config_server.client.requests.get")
+def test_read_unformatted_file_reading_not_OK(mock_request: MagicMock):
+    """Test that a non-200 response raises a RequestException."""
+    mock_request.return_value = make_test_response(
+        "1st_read", status.HTTP_204_NO_CONTENT, raise_exc=requests.exceptions.HTTPError
     )
+    file_path = "test"
+    url = "url"
+    server = ConfigServer(url)
+    with pytest.raises(requests.exceptions.HTTPError):
+        server.get_file_contents(file_path)