Skip to content

Commit 59a5153

Browse files
committed
Wait_for_comnfirmed returns True if assets/events do not exist
Problem: The filter criteria for the wait_for_confirmed method can return zero if the user has made a mistake and this will correspond to an erroneous True result. Solution: Check that the no of assets/events is > 0 before entering the wait_for loop, Signed-off-by: Paul Hewlett <phewlett76@gmail.com>
1 parent d719c7f commit 59a5153

5 files changed

Lines changed: 80 additions & 24 deletions

File tree

archivist/assets.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,10 @@
3636
from .constants import (
3737
ASSETS_SUBPATH,
3838
ASSETS_LABEL,
39+
CONFIRMATION_STATUS,
3940
)
40-
from .confirm import wait_for_confirmation, wait_for_confirmed
41+
from .confirm import _wait_for_confirmation, _wait_for_confirmed
42+
from .errors import ArchivistNotFoundError
4143

4244

4345
#: Default page size - number of entities fetched in one REST GET in the
@@ -164,7 +166,7 @@ def create_from_data(self, data: Dict, *, confirm: bool = False) -> Asset:
164166
if not confirm:
165167
return asset
166168

167-
return wait_for_confirmation(self, asset["identity"]) # type: ignore
169+
return _wait_for_confirmation(self, asset["identity"]) # type: ignore
168170

169171
def wait_for_confirmation(self, identity: str) -> bool:
170172
"""Wait for asset to be confirmed.
@@ -178,7 +180,7 @@ def wait_for_confirmation(self, identity: str) -> bool:
178180
True if asset is confirmed.
179181
180182
"""
181-
return wait_for_confirmation(self, identity)
183+
return _wait_for_confirmation(self, identity)
182184

183185
def read(self, identity: str) -> Asset:
184186
"""Read asset
@@ -236,7 +238,16 @@ def wait_for_confirmed(
236238
True if all assets are confirmed.
237239
238240
"""
239-
return wait_for_confirmed(self, props=props, attrs=attrs)
241+
# check that entities exist
242+
newprops = deepcopy(props) if props else {}
243+
newprops.pop(CONFIRMATION_STATUS, None)
244+
245+
LOGGER.debug("Count assets %s", newprops)
246+
count = self.count(props=newprops, attrs=attrs)
247+
if count == 0:
248+
raise ArchivistNotFoundError("No assets exist")
249+
250+
return _wait_for_confirmed(self, props=newprops, attrs=attrs)
240251

241252
def list(
242253
self,

archivist/confirm.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ def __on_giveup_confirmation(details):
5050
on_backoff=__backoff_handler,
5151
on_giveup=__on_giveup_confirmation,
5252
)
53-
def wait_for_confirmation(self, identity):
53+
def _wait_for_confirmation(self, identity):
5454
"""docstring"""
5555
entity = self.read(identity)
5656

@@ -89,12 +89,14 @@ def __on_giveup_confirmed(details):
8989
on_backoff=__backoff_handler,
9090
on_giveup=__on_giveup_confirmed,
9191
)
92-
def wait_for_confirmed(self, *, props=None, **kwargs) -> bool:
92+
def _wait_for_confirmed(self, *, props=None, **kwargs) -> bool:
9393
"""docstring"""
94+
95+
# look for unconfirmed entities
9496
newprops = deepcopy(props) if props else {}
9597
newprops[CONFIRMATION_STATUS] = CONFIRMATION_PENDING
9698

97-
LOGGER.debug("Count unconfirmed assets %s", newprops)
99+
LOGGER.debug("Count unconfirmed entities %s", newprops)
98100
count = self.count(props=newprops, **kwargs)
99101

100102
if count == 0:
@@ -103,7 +105,7 @@ def wait_for_confirmed(self, *, props=None, **kwargs) -> bool:
103105
newprops[CONFIRMATION_STATUS] = CONFIRMATION_FAILED
104106
count = self.count(props=newprops, **kwargs)
105107
if count > 0:
106-
raise ArchivistUnconfirmedError(f"There are {count} FAILED assets")
108+
raise ArchivistUnconfirmedError(f"There are {count} FAILED entities")
107109

108110
return True
109111

archivist/events.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,11 @@
3434
SEP,
3535
ASSETS_SUBPATH,
3636
ASSETS_WILDCARD,
37+
CONFIRMATION_STATUS,
3738
EVENTS_LABEL,
3839
)
39-
from .confirm import wait_for_confirmation, wait_for_confirmed
40+
from .confirm import _wait_for_confirmation, _wait_for_confirmed
41+
from .errors import ArchivistNotFoundError
4042

4143

4244
#: Default page size - number of entities fetched in one REST GET in the
@@ -172,7 +174,7 @@ def create_from_data(self, asset_id: str, data: Dict, *, confirm=False) -> Event
172174
if not confirm:
173175
return event
174176

175-
return wait_for_confirmation(self, event["identity"]) # type: ignore
177+
return _wait_for_confirmation(self, event["identity"]) # type: ignore
176178

177179
def wait_for_confirmation(self, identity: str) -> bool:
178180
"""Wait for event to be confirmed.
@@ -186,7 +188,7 @@ def wait_for_confirmation(self, identity: str) -> bool:
186188
True if event is confirmed.
187189
188190
"""
189-
return wait_for_confirmation(self, identity)
191+
return _wait_for_confirmation(self, identity)
190192

191193
def read(self, identity: str) -> Event:
192194
"""Read event
@@ -269,7 +271,18 @@ def wait_for_confirmed(
269271
True if all events are confirmed.
270272
271273
"""
272-
return wait_for_confirmed(
274+
# check that entities exist
275+
newprops = deepcopy(props) if props else {}
276+
newprops.pop(CONFIRMATION_STATUS, None)
277+
278+
LOGGER.debug("Count events %s", newprops)
279+
count = self.count(
280+
asset_id=asset_id, props=newprops, attrs=attrs, asset_attrs=asset_attrs
281+
)
282+
if count == 0:
283+
raise ArchivistNotFoundError("No events exist")
284+
285+
return _wait_for_confirmed(
273286
self, asset_id=asset_id, props=props, attrs=attrs, asset_attrs=asset_attrs
274287
)
275288

unittests/testassets.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
HEADERS_TOTAL_COUNT,
1717
ROOT,
1818
)
19-
from archivist.errors import ArchivistUnconfirmedError
19+
from archivist.errors import ArchivistNotFoundError, ArchivistUnconfirmedError
2020
from archivist.storage_integrity import StorageIntegrity
2121

2222
from .mock_response import MockResponse
@@ -415,7 +415,7 @@ def test_assets_wait_for_confirmed(self):
415415
Test asset counting
416416
"""
417417
## last call to get looks for FAILED assets
418-
status = ("PENDING", "PENDING", "FAILED")
418+
status = ("", "&confirmation_status=PENDING", "&confirmation_status=FAILED")
419419
with mock.patch.object(self.arch._session, "get") as mock_get:
420420
mock_get.side_effect = [
421421
MockResponse(
@@ -442,13 +442,7 @@ def test_assets_wait_for_confirmed(self):
442442
self.assertEqual(
443443
tuple(a),
444444
(
445-
(
446-
(
447-
f"url/{ROOT}/{SUBPATH}"
448-
"?page_size=1"
449-
f"&confirmation_status={status[i]}"
450-
),
451-
),
445+
((f"url/{ROOT}/{SUBPATH}" "?page_size=1" f"{status[i]}"),),
452446
{
453447
"headers": {
454448
"content-type": "application/json",
@@ -462,6 +456,24 @@ def test_assets_wait_for_confirmed(self):
462456
msg="GET method called incorrectly",
463457
)
464458

459+
def test_assets_wait_for_confirmed_not_found(self):
460+
"""
461+
Test asset counting
462+
"""
463+
with mock.patch.object(self.arch._session, "get") as mock_get:
464+
mock_get.side_effect = [
465+
MockResponse(
466+
200,
467+
headers={HEADERS_TOTAL_COUNT: 0},
468+
assets=[
469+
RESPONSE_PENDING,
470+
],
471+
),
472+
]
473+
474+
with self.assertRaises(ArchivistNotFoundError):
475+
self.arch.assets.wait_for_confirmed()
476+
465477
def test_assets_wait_for_confirmed_timeout(self):
466478
"""
467479
Test asset counting

unittests/testevents.py

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
HEADERS_REQUEST_TOTAL_COUNT,
1818
HEADERS_TOTAL_COUNT,
1919
)
20-
from archivist.errors import ArchivistUnconfirmedError
20+
from archivist.errors import ArchivistNotFoundError, ArchivistUnconfirmedError
2121
from archivist.events import Event, DEFAULT_PAGE_SIZE
2222

2323
from .mock_response import MockResponse
@@ -651,7 +651,7 @@ def test_events_wait_for_confirmed(self):
651651
Test event counting
652652
"""
653653
## last call to get looks for FAILED assets
654-
status = ("PENDING", "PENDING", "FAILED")
654+
status = ("", "&confirmation_status=PENDING", "&confirmation_status=FAILED")
655655
with mock.patch.object(self.arch._session, "get") as mock_get:
656656
mock_get.side_effect = [
657657
MockResponse(
@@ -684,7 +684,7 @@ def test_events_wait_for_confirmed(self):
684684
f"/{ASSETS_WILDCARD}"
685685
f"/{EVENTS_LABEL}"
686686
"?page_size=1"
687-
f"&confirmation_status={status[i]}"
687+
f"{status[i]}"
688688
),
689689
),
690690
{
@@ -700,6 +700,24 @@ def test_events_wait_for_confirmed(self):
700700
msg="GET method called incorrectly",
701701
)
702702

703+
def test_events_wait_for_confirmed_not_found(self):
704+
"""
705+
Test event counting
706+
"""
707+
with mock.patch.object(self.arch._session, "get") as mock_get:
708+
mock_get.side_effect = [
709+
MockResponse(
710+
200,
711+
headers={HEADERS_TOTAL_COUNT: 0},
712+
assets=[
713+
RESPONSE_PENDING,
714+
],
715+
),
716+
]
717+
718+
with self.assertRaises(ArchivistNotFoundError):
719+
self.arch.events.wait_for_confirmed()
720+
703721
def test_events_list(self):
704722
"""
705723
Test event listing

0 commit comments

Comments
 (0)