feat: add async concurrency helpers - #103
Conversation
There was a problem hiding this comment.
Pull request overview
Adds asyncio-based concurrency helpers to pyscrappy.concurrent to mirror the existing threadpool-backed helpers, enabling native async callers to run many scrapes concurrently with bounded parallelism.
Changes:
- Added
scrape_many_async()to runBaseScraper.scrape_async()calls concurrently with anasyncio.Semaphore. - Added
scrape_all_async()to run multiple async scrape callables concurrently, preserving input order. - Extended
tests/test_concurrent.pywith async test coverage for ordering, empty input, and concurrent execution.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/test_concurrent.py | Adds async tests for the new async concurrency helpers alongside existing sync tests. |
| src/pyscrappy/concurrent.py | Introduces scrape_many_async / scrape_all_async using asyncio + semaphore to bound concurrency and gather to preserve order. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (6)
src/pyscrappy/concurrent.py:117
- For consistency with
scrape_all/scrape_many,scrape_all_async’s docstring should document ordering, the concurrency cap, and the expected callable shape (zero-arg callable returning an awaitableScrapeResult).
"""Run several independent async scrape callables concurrently."""
src/pyscrappy/concurrent.py:127
- Same exception-handling concern as
scrape_many_async: if one callable fails,gatherraises immediately and other tasks continue running in the background. Cancel/await the remaining tasks so the function doesn’t return while work is still in flight.
tasks = [asyncio.create_task(_one(f)) for f in funcs]
return await asyncio.gather(*tasks)
tests/test_concurrent.py:126
- Similarly,
TestScrapeAllAsync.test_concurrentusesmax_concurrency=4for 4 tasks, which doesn’t exercise the semaphore’s cap. Consider using a smallermax_concurrencyand asserting the runtime reflects batching.
await scrape_all_async(funcs, max_concurrency=4)
src/pyscrappy/concurrent.py:87
- If any scrape task raises,
asyncio.gather(*tasks)will propagate the exception immediately while the other tasks keep running in the background. That can leave in-flight scrapes (and theirasync withcleanup) detached from the caller and potentially running after the API has already errored. Consider canceling remaining tasks and awaiting them (withreturn_exceptions=True) before re-raising.
This issue also appears on line 126 of the same file.
tasks = [asyncio.create_task(_one(call)) for call in calls]
return await asyncio.gather(*tasks)
src/pyscrappy/concurrent.py:76
- The sync helpers in this module use full docstrings (Args/Returns and behavioral notes). For consistency and to document important guarantees (order preservation, per-call scraper lifecycle, concurrency cap),
scrape_many_asyncshould include the same level of detail.
This issue also appears on line 117 of the same file.
"""Run ``scraper_cls.scrape_async(**call)`` for each call concurrently."""
tests/test_concurrent.py:108
- The async concurrency tests only cover the fully-parallel case (
max_concurrency == len(calls)), so they don’t verify that the semaphore actually caps concurrency whenmax_concurrencyis smaller than the work size (one of the main acceptance criteria).
This issue also appears on line 126 of the same file.
max_concurrency=4,
f83827e to
83435a0
Compare
…cy caps Add the anyio_backend='asyncio' fixture the other async test files use, so these tests don't run (and fail) under trio. Add a counter-based test proving max_concurrency actually limits in-flight scrapes (the existing timing tests used max_concurrency == len(calls), which passes even if the semaphore is a no-op — mldsveda#71 asked for a real cap).
|
Nice work, the helpers mirror the sync ones cleanly. Pushed a test follow-up: pinned the anyio backend to asyncio (matching the other async test files, otherwise these fail under trio) and added a counter-based test proving max_concurrency actually caps in-flight scrapes, since the timing tests passed either way. Green now, thanks @Siteshcodes! |
Summary
Adds native async concurrency helpers that mirror the existing synchronous helpers.
Changes
scrape_many_async()scrape_all_async()asyncio.gather()asyncio.SemaphoreValidation
python -m pytestFixes #71