fix: loop over paginated proposals (#459)

Author: minottic

importTools/update_locations.py

@@ -161,7 +161,7 @@ def __init__(self, name):
 
     scilog = ClientSettingsFromEnv("SCILOG")
 
-    cat = SciCat(**scicat.__dict__)
+    cat = SciCat(**scicat.__dict__, return_options={"lazy": True})
     props = cat.proposals
 
     log = SciLog(**scilog.__dict__)

sdk/python/scilog/scicat.py

@@ -1,3 +1,6 @@
+from json import dumps
+from urllib.parse import quote
+
 from .authmixin import HEADER_JSON, AuthError
 from .httpclient import HttpClient
 
@@ -20,13 +23,34 @@ def authenticate(self, username, password):
 
 
 class SciCat:
-    def __init__(self, *args, **kwargs):
+    max_iterations = 1000
+
+    def __init__(self, *args, return_options=None, **kwargs):
+        self.return_options = return_options or {}
         self.http_client = SciCatRestAPI(*args, **kwargs)
 
+    def _proposals_batch(self):
+        url = self.http_client.address + "/proposals"
+        limit = 500
+        filter = {"limits": {"skip": 0, "limit": limit}}
+        iteration = 0
+        while True:
+            iteration += 1
+            if iteration > self.max_iterations:
+                raise RuntimeError("Exceeded maximum iterations in proposals_batch")
+            proposals = self.http_client.get_request(
+                f"{url}?filter={quote(dumps(filter))}", headers=HEADER_JSON
+            )
+            if not proposals or len(proposals) == 0:
+                break
+            yield proposals
+            filter["limits"]["skip"] += limit
+
     @property
     def proposals(self):
-        url = self.http_client.address + "/proposals"
-        return self.http_client.get_request(url, headers=HEADER_JSON)
+        lazy = self.return_options.get("lazy", False)
+        generator = (proposal for batch in self._proposals_batch() for proposal in batch)
+        return generator if lazy else list(generator)
 
 
 class SciCatAuthError(AuthError):

sdk/python/tests/test_scicat.py

@@ -1,9 +1,13 @@
-from unittest.mock import ANY, Mock, patch
+from json import dumps
+from unittest.mock import ANY, Mock, call, patch
+from urllib.parse import quote
 
 import pytest
 
 from scilog import SciCat
 
+ADDRESS = "http://scicat"
+
 
 @patch("requests.post")
 @patch("requests.get")
@@ -16,33 +20,73 @@
     ],
 )
 def test_get_proposals(mock_get, mock_post, token_prefix):
-    address = "http://scicat"
     options = {
         "username": f"username{token_prefix}",
         "password": "password",
-        "login_path": f"{address}/login",
+        "login_path": f"{ADDRESS}/login",
         "token_prefix": token_prefix,
     }
     headers = {"Content-type": "application/json", "Accept": "application/json"}
     token = "token123"
 
-    scicat = SciCat(address, options=options)
+    scicat = SciCat(ADDRESS, options=options)
+    scicat.http_client.config = {}
     mock_response = Mock()
     mock_response.json.return_value = {"id": token}
     mock_post.return_value = mock_response
-    scicat.http_client.config = {}
-    scicat.proposals
+    list(scicat.proposals)
     mock_post.assert_called_with(
         options["login_path"],
         json={"username": options["username"], "password": options["password"]},
         headers=headers,
         timeout=ANY,
         verify=True,
     )
+    filter = {"limits": {"skip": 0, "limit": 500}}
     mock_get.assert_called_with(
-        f"{address}/proposals",
+        f"{ADDRESS}/proposals?filter={quote(dumps(filter))}",
         params=None,
         headers={**headers, "Authorization": f"{token_prefix or ''}{token}"},
         timeout=ANY,
         verify=True,
     )
+
+
+@patch("scilog.scicat.SciCatRestAPI.get_request")
+def test__proposals_batch(mock_get):
+    scicat = SciCat(ADDRESS)
+    scicat.http_client.config = {}
+
+    mock_get.side_effect = [[1, 2], [3, 4], []]
+    filters = [{"limits": {"skip": 0, "limit": 500}}, {"limits": {"skip": 500, "limit": 500}}]
+    for _ in scicat._proposals_batch():
+        continue
+
+    assert mock_get.call_count == 3
+
+    expected_calls = [
+        call(f"{ADDRESS}/proposals?filter={quote(dumps(filters[0]))}", headers=ANY),
+        call(f"{ADDRESS}/proposals?filter={quote(dumps(filters[1]))}", headers=ANY),
+    ]
+    mock_get.assert_has_calls(expected_calls, any_order=False)
+
+
+@patch("scilog.scicat.SciCatRestAPI.get_request")
+@pytest.mark.parametrize(
+    "return_options",
+    [
+        None,
+        {"lazy": True},
+        {"lazy": False},
+    ],
+)
+def test_proposals(mock_get, return_options):
+    scicat = SciCat(ADDRESS, return_options=return_options)
+    scicat.http_client.config = {}
+    mock_get.side_effect = [[1, 2], [3, 4], []]
+    proposals = [1, 2, 3, 4]
+    scicat_proposals = scicat.proposals
+    for i, p in enumerate(scicat_proposals):
+        assert p == proposals[i]
+    lazy = return_options.get("lazy", False) if return_options else False
+    assert len(list(scicat_proposals)) == (0 if lazy else len(proposals))