fix: Add is_finished and align is_empty semantics in ApifyRequestQueueClient#1008
fix: Add is_finished and align is_empty semantics in ApifyRequestQueueClient#1008Mantisus wants to merge 2 commits into
is_finished and align is_empty semantics in ApifyRequestQueueClient#1008Conversation
| async def is_finished(self) -> bool: | ||
| """Specific implementation of this method for the RQ shared access mode.""" | ||
| async with self._fetch_lock: | ||
| return await self._is_empty() and not self._queue_has_locked_requests |
There was a problem hiding this comment.
This relies on await self._is_empty() is being evaluated first (it calls _list_head, which populates self._queue_has_locked_requests). A reorder would break the finished detection. Maybe we could add a comment noting the ordering dependency.
| @@ -279,7 +279,11 @@ async def is_empty(self) -> bool: | |||
| """Specific implementation of this method for the RQ single access mode.""" | |||
| # Without the lock the `is_empty` is prone to falsely report True with some low probability race condition. | |||
There was a problem hiding this comment.
This comment is now outdated and misleading. Could you update it?
| assert metadata.pending_request_count == 0, f'metadata.pending_request_count={metadata.pending_request_count}' | ||
|
|
||
|
|
||
| async def test_is_empty_and_is_finished(request_queue_apify: RequestQueue, rq_poll_timeout: int) -> None: |
There was a problem hiding this comment.
For the most part, this duplicates the existing test_request_queue_is_finished test. Could we consolidate them into one? And parametrized it single+shared.
vdusek
left a comment
vdusek
left a comment
There was a problem hiding this comment.
Could we also use a fix here please? Same reason as explained here apify/crawlee-python#1982 (review)
Claude suggests one of these:
- fix: prevent RQ softlock by correcting is_empty in ApifyRequestQueueClient
- fix: add is_finished and align is_empty semantics in ApifyRequestQueueClient
I am ok with both of them
Mantisus
changed the title
is_finished method in the ApifyStorageClientis_finished and align is_empty semantics in ApifyRequestQueueClient
3 participants
Description
Add
is_finishedand alignis_emptysemantics inApifyRequestQueueClientCloses: #987
Follow-up: apify/crawlee-python#1982