refactor: adopt asyncio.TaskGroup for structured concurrency#909
Draft
vdusek wants to merge 3 commits into
Draft
refactor: adopt asyncio.TaskGroup for structured concurrency#909vdusek wants to merge 3 commits into
vdusek wants to merge 3 commits into
Conversation
Switch from asyncio.gather to asyncio.TaskGroup in two places enabled by the Python 3.11+ baseline: - ApifyRequestList._fetch_requests_from_url: structured task scope so any fetch failure cancels siblings cleanly and aggregates errors in an ExceptionGroup instead of swallowing all but the first. - Actor.reboot pre-reboot dispatch: combine asyncio.timeout + TaskGroup, with each listener wrapped to preserve the existing best-effort semantics (one failing listener does not abort the others). Closes #765.
vdusek
added
adhoc
vdusek
added
adhoc
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## drop-python-3.10 #909 +/- ##
====================================================
- Coverage 87.05% 87.00% -0.05%
====================================================
Files 48 48
Lines 2943 2956 +13
====================================================
+ Hits 2562 2572 +10
- Misses 381 384 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
`_save_actor_state` previously iterated `_use_state_stores` and awaited each `persist_autosaved_values` call sequentially. The persists are independent — fan them out with `asyncio.TaskGroup` so cleanup latency is bounded by the slowest store instead of summed across stores. A per-task try/except keeps the cleanup best-effort: one failing store no longer prevents siblings from persisting.
`ApifyFileSystemKeyValueStoreClient.purge` previously awaited each `unlink` sequentially. The `asyncio.to_thread` calls are independent — fan them out with `asyncio.TaskGroup` so purge latency scales with the slowest deletion instead of the total file count.
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
2 participants
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.
Stacked on top of #908 — base will retarget to
masterautomatically once #908 merges.Adopts
asyncio.TaskGroup(Python 3.11+) in four places:ApifyRequestList._fetch_requests_from_url— structured task scope for concurrent remote-list fetches; errors aggregate asExceptionGroup.Actor.rebootpre-reboot dispatch —asyncio.timeout(...)+TaskGroup, each listener wrapped insafe_dispatchso one failure no longer aborts siblings (preservestest_reboot_runs_all_listeners_even_when_one_fails).Actor._save_actor_state— per-store persists fan out instead of running sequentially; per-task try/except keeps cleanup best-effort.ApifyFileSystemKeyValueStoreClient.purge—unlinkcalls fan out instead of running sequentially.Skipped:
_apify_event_manager.py(Low Priority — #765 itself flags it as needing class restructuring) andActor.__aenter__(event/charging manager init) — the charging manager's__aenter__sets aContextVarthat doesn't propagate out of a child task, so parallelizing silently breaks PPE charging.Closes #765.