fix: align URL pattern matching and limit semantics with crawlee-js - #2166
Open
atirna wants to merge 1 commit into
Open
fix: align URL pattern matching and limit semantics with crawlee-js#2166atirna wants to merge 1 commit into
atirna wants to merge 1 commit into
Conversation
- Glob patterns now match case-insensitively (Minimatch nocase: true) - Regex include/exclude patterns match unanchored, like regexp.test in JS - enqueue_links limit counts newly enqueued requests, applied after transform_request_function and skipping duplicates already in the queue - SitemapRequestLoader regex patterns follow the same unanchored semantics Fixes apify#2122
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Aligns
enqueue_links/add_requestsURL filtering with crawlee-js, following up on the differences found in the apify/crawlee#3533 review (thanks @vdusek for the detailed writeup).Globpatterns now match case-insensitively, likeMinimatchwithnocase: truein JS (_utils/globs.py)include/excludepatterns are matched withsearchinstead ofmatch, so they match anywhere in the URL likeregexp.testin JS (crawler +SitemapRequestLoader, which mirrorsurl.matchon the JS side)limitnow counts requests actually enqueued: applied last, aftertransform_request_functionskipping, and duplicates already present in the queue don't consume it (JS decrements itsmaxNewRequestsbudget only for requests that weren't already there)One thing worth calling out since @B4nan wanted more opinions on the regexp anchoring: I checked the JS side, and both
enqueueLinks(createPatternObjectMatcherinpackages/core/src/enqueue_links/shared.ts) and the sitemap loader (url.match(regexp)inpackages/core/src/storages/sitemap_request_loader.ts) search unanchored, so a pattern like/\/products\//matching anywhere in the URL is the established cross-library behavior. That's what this implements.Issues
Testing
before, on current
master(172699b1):Glob('https://Someplace.com/**/cats')did not matchhttps://someplace.com/blog/category/catsinclude=[re.compile(r'/category/cats')]enqueued nothing (the regex never matched at the start of the URL)limit=2and a transform skipping 2 of 3 extracted links, the remaining link was not enqueued (limit burned slots on skipped URLs); URLs already in the queue also consumed the limitafter:
uv run pytest tests/unit/_utils/test_globs.py tests/unit/crawlers/_basic tests/unit/crawlers/_beautifulsoup tests/unit/request_loaders/test_sitemap_request_loader.py— new regression tests for all of the above (each fails onmaster, passes here), existing suite unaffecteduv run poe unit-tests— 2296 passed, 11 skippeduv run poe lint/uv run poe type-check— cleanChecklist