Skip to content

Commit

Permalink
set total_pages_api to None and avoid warnings in multiquery use case
Browse files Browse the repository at this point in the history
  • Loading branch information
leoschwarz committed May 17, 2024
1 parent bb71a4b commit 9ae1ac9
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 40 deletions.
6 changes: 3 additions & 3 deletions bfabric/experimental/multi_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def read_multi(
# TODO: It is assumed that a user requesting multi_query always wants all of the pages. Can anybody think of
# exceptions to this?
response_this = self._client.read(endpoint, obj_extended, max_results=None, return_id_only=return_id_only)
response_tot.extend(response_this)
response_tot.extend(response_this, reset_total_pages_api=True)

return response_tot

Expand All @@ -68,7 +68,7 @@ def read_multi(
# # Iterate over request chunks that fit into a single API page
# for page_objs in page_iter(obj_lst):
# response_page = self.save(endpoint, page_objs, **kwargs)
# response_tot.extend(response_page)
# response_tot.extend(response_page, reset_total_pages_api=True
#
# return response_tot

Expand All @@ -85,7 +85,7 @@ def delete_multi(self, endpoint: str, id_list: list[int]) -> ResultContainer:
# Iterate over request chunks that fit into a single API page
for page_ids in page_iter(id_list):
response_page = self._client.delete(endpoint, page_ids)
response_tot.extend(response_page)
response_tot.extend(response_page, reset_total_pages_api=True)

return response_tot

Expand Down
13 changes: 7 additions & 6 deletions bfabric/results/result_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,16 @@ def errors(self) -> list:
"""List of errors that occurred during the query. An empty list means the query was successful."""
return self._errors

def extend(self, other: ResultContainer) -> None:
"""
Can merge results of two queries. This can happen if the engine splits a complicated query in two
:param other: The other query results that should be appended to this
:return:
def extend(self, other: ResultContainer, reset_total_pages_api: bool = False) -> None:
"""Merges the results of `other` into this container.
:param other: The container whose elements to append to the end of this container
:param reset_total_pages_api: If True, the total_pages_api attribute will be reset to None
"""
self.results += other.results
self._errors += other.errors
if self._total_pages_api != other.total_pages_api:
if reset_total_pages_api:
self._total_pages_api = None
elif self._total_pages_api != other.total_pages_api:
logging.warning(
f"Results observed with different total pages counts: "
f"{self._total_pages_api} != {other.total_pages_api}"
Expand Down
65 changes: 34 additions & 31 deletions bfabric/tests/integration/test_multi_methods.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import json
import os
import unittest
from pathlib import Path

from bfabric import Bfabric, BfabricAPIEngineType
from bfabric.experimental.multi_query import MultiQuery
Expand All @@ -9,8 +9,8 @@
class BfabricTestMulti(unittest.TestCase):
def setUp(self, *args, **kwargs):
# Load ground truth
path = os.path.join(os.path.dirname(__file__), "groundtruth.json")
with open(path) as json_file:
path = Path(__file__).parent / "groundtruth.json"
with path.open() as json_file:
self.ground_truth = json.load(json_file)

# Create clients
Expand All @@ -20,41 +20,42 @@ def setUp(self, *args, **kwargs):
}

def _test_multi_read_delete(self, engine: str):
"""
Create many workunits
"""Creates many workunits
* Test if reading multiple of those workunits works
* Test if exists on multiple workunits works
* Test if deleting multiple workunits works
"""
with self.subTest(engine=engine):
bf: Bfabric = self.clients[engine]
mq = MultiQuery(bf)

# 1. Create a bunch of workunits
# Note: we crate more than 100, to make sure pagination works correctly
n_units = 105
workunit_ids = []
for i in range(n_units):
query = {"name": "fancy_workunit_"+str(i), "applicationid": 2, "description": "is very fancy", "containerid": 3000}
res = bf.save("workunit", query).to_list_dict()
self.assertEqual(len(res), 1)
self.assertIn("id", res[0])
workunit_ids += [res[0]['id']]
bf: Bfabric = self.clients[engine]
mq = MultiQuery(bf)

# 1. Create a bunch of workunits
# Note: we crate more than 100, to make sure pagination works correctly
n_units = 105
workunit_ids = []
for i in range(n_units):
query = {
"name": f"fancy_workunit_{i}",
"applicationid": 2,
"description": "is very fancy",
"containerid": 3000,
}
res = bf.save("workunit", query).to_list_dict()
self.assertEqual(len(res), 1)
self.assertIn("id", res[0])
workunit_ids += [res[0]["id"]]

#2. TODO: Make sure that the results are indeed read correctly, not just read
res = mq.read_multi('workunit', {}, 'id', workunit_ids, return_id_only=True)
# 2. TODO: Make sure that the results are indeed read correctly, not just read
res = mq.read_multi("workunit", {}, "id", workunit_ids, return_id_only=True)

#3. Check if correct ones exist and fake one does not
res = mq.exists_multi('workunit', 'id', workunit_ids + [10101010101010])
self.assertEqual(len(res), n_units + 1)
self.assertTrue(all(res[:n_units]))
self.assertFalse(res[n_units])

# 4. Delete all workunits at the same time
res = mq.delete_multi('workunit', workunit_ids)
self.assertEqual(len(res), n_units)
# 3. Check if correct ones exist and fake one does not
res = mq.exists_multi("workunit", "id", workunit_ids + [10101010101010])
self.assertEqual(len(res), n_units + 1)
self.assertTrue(all(res[:n_units]))
self.assertFalse(res[n_units])

# 4. Delete all workunits at the same time
res = mq.delete_multi("workunit", workunit_ids)
self.assertEqual(len(res), n_units)

# TODO: Implement me
def _test_multi_read_complex(self, engine: str):
Expand All @@ -67,8 +68,10 @@ def _test_multi_read_complex(self, engine: str):
"""
pass

def test_multi_delete(self):
def test_multi_delete_when_suds(self):
self._test_multi_read_delete("suds")

def test_multi_delete_when_zeep(self):
self._test_multi_read_delete("zeep")


Expand Down

0 comments on commit 9ae1ac9

Please sign in to comment.