Skip to content

fix: Make result of RequestList.is_empty independent of fetch_next_request calls#876

Merged
janbuchar merged 2 commits into
masterfrom
fix-request-list-empty
Jan 7, 2025
Merged

fix: Make result of RequestList.is_empty independent of fetch_next_request calls#876
janbuchar merged 2 commits into
masterfrom
fix-request-list-empty

Conversation

@janbuchar

Copy link
Copy Markdown
Collaborator

The old version of RequestList only updated the _is_empty flag on fetch_next_request calls, which was not enough. This PR updates it so that the is_empty method also tries to dequeue a request from the iterator before returning a result.

@janbuchar janbuchar added t-tooling Issues with this label are in the ownership of the tooling team. adhoc Ad-hoc unplanned task added during the sprint. labels Jan 6, 2025
@janbuchar janbuchar requested review from Pijukatel and vdusek January 6, 2025 16:46
@github-actions github-actions Bot added this to the 105th sprint - Tooling team milestone Jan 6, 2025
Comment thread src/crawlee/request_loaders/_request_list.py

@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.

Would it be possible to add a test case that covers the issue from the SDK?

@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Jan 7, 2025
@janbuchar janbuchar requested a review from vdusek January 7, 2025 12:55

@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.

LGTM

@janbuchar janbuchar merged commit d50249e into master Jan 7, 2025
@janbuchar janbuchar deleted the fix-request-list-empty branch January 7, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants