Skip to content

refactor(request-loaders)!: Replace is_empty and is_finished with check_readiness - #2334

Open
Mantisus wants to merge 4 commits into
apify:v2from
Mantisus:check-readiness
Open

Mantisus wants to merge 4 commits into
apify:v2from
Mantisus:check-readiness

Conversation

@Mantisus

@Mantisus Mantisus commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Description

RequestLoader and RequestManager replace is_empty() and is_finished() with a single check_readiness(). It returns one of four statuses, exported from crawlee.request_loaders: ReadyStatus, WaitingStatus, StalledStatus or FinishedStatus. WaitingStatus.ready_at says when a source expects to have work again, and ThrottlingRequestManager sets it from its cooldowns. StalledStatus.reason says why a source can't make progress. Storage clients keep is_empty() and is_finished(). The upgrade guide covers the migration.

  • ThrottlingRequestManager.get_stall_reason() is removed. The crawler reads a stall from the status, so it now also detects a stalled domain when the ThrottlingRequestManager is wrapped in a RequestManagerTandem.
  • RequestManagerTandem moves a request from its loader only when the loader is ready, so a SitemapRequestLoader that's still loading sitemaps no longer holds up the requests already in the manager.
  • RequestLoader.fetch_next_request() no longer has to wait for a request. A None means nothing is pending right now, not that the loader is finished. SitemapRequestLoader.fetch_next_request() returns None once its sitemaps are loaded, instead of waiting until every fetched request is handled.

Issues

✍️ Drafted by Claude Code

@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Oct 15, 2026
@Mantisus Mantisus self-assigned this Oct 15, 2026
@Mantisus
Mantisus marked this pull request as ready for review October 15, 2026 21:59
Comment thread src/crawlee/crawlers/_basic/_basic_crawler.py Outdated
Comment thread docs/examples/code_examples/run_parallel_crawlers.py Outdated
Comment on lines +75 to +85
```python
from crawlee.request_loaders import FinishedStatus, ReadyStatus

# v1
has_requests = not await rq.is_empty()
done = await rq.is_finished()

# v2
has_requests = isinstance(await rq.check_readiness(), ReadyStatus)
done = isinstance(await rq.check_readiness(), FinishedStatus)
```

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned above, this is much less convenient than before. You now need an additional import and an isinstance() check.

Maybe we could introduce helper methods for checking statuses, so determining whether an RQ is in a particular status would be a single method call?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, then let's use literal comparison at least, so users aren't pushed into an extra import

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janbuchar Have you considered keeping is_finished as a simple helper that wraps

isinstance(await rq.check_readiness(), FinishedStatus)

?

We could add similar helpers for the other statuses where they make sense.

I mean, the new API is a bit worse in terms of ergonomics (maybe with AI agents it doesn't matter that much?).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, then let's use literal comparison at least, so users aren't pushed into an extra import

Implemented in 883a68c

@Mantisus
Mantisus force-pushed the check-readiness branch 2 times, most recently from 3629140 to a30518d Compare October 18, 2026 17:01
@vdusek
vdusek requested review from Pijukatel and vdusek October 19, 2026 11:00

@vdusek vdusek left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one potential optimization note


request_manager = await self.get_request_manager()
return not await request_manager.is_empty()
return (await request_manager.check_readiness()).status == 'ready'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The pool calls __is_finished_function and then __is_task_ready_function on every pass, and both now run check_readiness(). On a queue that is empty but not finished, each call makes two backend calls (is_empty() + is_finished()), so a pass costs 4 calls where it used to cost 2. That covers the end of a crawl, waiting on in-progress requests, and throttler cooldowns. On the platform these are API calls, and ThrottlingRequestManager repeats them for every domain queue. Could the status from __is_finished_function be reused here, or is that too tied to the call order in the pool?

✍️ Drafted by Claude Code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be resolved in #2332

Comment thread src/crawlee/request_loaders/_request_manager_tandem.py Outdated
Comment thread src/crawlee/request_loaders/_request_manager_tandem.py Outdated
Comment thread src/crawlee/request_loaders/_types.py Outdated
Comment on lines +22 to +27
if first.status == 'waiting':
if second.status != 'waiting' or second.ready_at is None:
return first
if first.ready_at is not None and first.ready_at <= second.ready_at:
return first
return second

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: a waiting status with ready_at=None means requests are in progress or being added, so work can show up any moment. Joining it with ready_at=T returns T, which promises nothing before T. Nothing reads ready_at yet, but it's public. Shouldn't None win here? I see test_join_keeps_known_ready_at pins the current behavior, so maybe I'm missing the intent.

✍️ Drafted by Claude Code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's intentional, and JS joins them the same way ("the earlier of the two known wake-up times, unknown only if neither source announced one"). ready_at isn't a promise that nothing shows up before it. It says when a wake-up is worth scheduling, on top of the regular polling AutoscaledPool does anyway. I added a comment to the join and clarified the ready_at docstring.

Comment on lines +356 to +362
stalled = [
domain
for domain, readiness in zip(candidates, candidate_statuses, strict=True)
if readiness.status == 'ready'
]

if stalled:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: a stall wins here even if a candidate's inner queue is waiting only because a background add_requests batch is still running, so an idle crawler can raise PersistentRateLimitError while new requests are arriving. The old get_stall_reason did the same, so this isn't a regression and is fine as a follow-up.

✍️ Drafted by Claude Code

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I fixed it here, since this PR is what sets the order between the statuses.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants