Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-124309: Modernize thestaggered_race implementation to support eager task factories#124390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@ZeroIntensity
Copy link
Member

@ZeroIntensityZeroIntensity commentedSep 23, 2024
edited by bedevere-appbot
Loading

Copy link
Contributor

@graingertgraingert left a comment

Choose a reason for hiding this comment

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

You're using private methods of TaskGroup and starting tasks on the loop rather than the TaskGroup

@ZeroIntensity
Copy link
MemberAuthor

I think I'm just going to refactor this to not useTaskGroup. It's causing more problems than solutions.

@graingert
Copy link
Contributor

I think it's worth persevering with TaskGroup, you just need to write it without using add_done_callback or private attributes

@ZeroIntensity
Copy link
MemberAuthor

I'll try it, but I'm worried that it isn't possible when considering an eager task factory. The previous implementation used a variation of a task group (a list containing tasks, since it predatedasyncio.TaskGroup) but broke due to the reliance of tasks not starting before the event loop is called.

While we're here,staggered_race is undocumented -- might that be something worth addressing in this PR?

@graingert
Copy link
Contributor

A demo of what I mean wrt TaskGroup:

"""Support for running coroutines in parallel with staggered start times."""__all__='staggered_race',from .importlocksfrom .importtasksfrom .importtaskgroupsasyncdefstaggered_race(coro_fns,delay,*,loop=None):"""Run coroutines with staggered start times and take the first to finish.    This method takes an iterable of coroutine functions. The first one is    started immediately. From then on, whenever the immediately preceding one    fails (raises an exception), or when *delay* seconds has passed, the next    coroutine is started. This continues until one of the coroutines complete    successfully, in which case all others are cancelled, or until all    coroutines fail.    The coroutines provided should be well-behaved in the following way:    * They should only ``return`` if completed successfully.    * They should always raise an exception if they did not complete      successfully. In particular, if they handle cancellation, they should      probably reraise, like this::        try:            # do work        except asyncio.CancelledError:            # undo partially completed work            raise    Args:        coro_fns: an iterable of coroutine functions, i.e. callables that            return a coroutine object when called. Use ``functools.partial`` or            lambdas to pass arguments.        delay: amount of time, in seconds, between starting coroutines. If            ``None``, the coroutines will run sequentially.        loop: the event loop to use.    Returns:        tuple *(winner_result, winner_index, exceptions)* where        - *winner_result*: the result of the winning coroutine, or ``None``          if no coroutines won.        - *winner_index*: the index of the winning coroutine in          ``coro_fns``, or ``None`` if no coroutines won. If the winning          coroutine may return None on success, *winner_index* can be used          to definitively determine whether any coroutine won.        - *exceptions*: list of exceptions returned by the coroutines.          ``len(exceptions)`` is equal to the number of coroutines actually          started, and the order is the same as in ``coro_fns``. The winning          coroutine's entry is ``None``.    """# TODO: when we have aiter() and anext(), allow async iterables in coro_fns.winner_result=Nonewinner_index=Noneexceptions= []class_Done(Exception):passasyncdefrun_one_coro(this_index,coro_fn,this_failed):try:result=awaitcoro_fn()except (SystemExit,KeyboardInterrupt):raiseexceptBaseExceptionase:exceptions[this_index]=ethis_failed.set()# Kickstart the next coroutineelse:# Store winner's resultsnonlocalwinner_index,winner_result# There could be more than one winnerwinner_index=this_indexwinner_result=resultraise_Donetry:asyncwithtaskgroups.TaskGroup()astg:forthis_index,coro_fninenumerate(coro_fns):this_failed=locks.Event()exceptions.append(None)tg.create_task(run_one_coro(this_index,coro_fn,this_failed))try:awaittasks.wait_for(this_failed.wait(),delay)exceptTimeoutError:pass    except*_Done:passreturnwinner_result,winner_index,exceptions
ZeroIntensity reacted with eyes emoji

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Copy link
Contributor

@willingcwillingc left a comment

Choose a reason for hiding this comment

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

ZeroIntensityand others added4 commitsSeptember 25, 2024 20:53
Co-authored-by: Carol Willing <carolcode@willingconsulting.com>
@kumaraditya303kumaraditya303enabled auto-merge (squash)September 26, 2024 05:04
@kumaraditya303kumaraditya303 merged commitde929f3 intopython:mainSep 26, 2024
36 checks passed
@miss-islington-app
Copy link

Thanks@ZeroIntensity for the PR, and@kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestSep 26, 2024
…port eager task factories (pythonGH-124390)(cherry picked from commitde929f3)Co-authored-by: Peter Bierma <zintensitydev@gmail.com>Co-authored-by: Thomas Grainger <tagrain@gmail.com>Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>Co-authored-by: Carol Willing <carolcode@willingconsulting.com>Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@miss-islington-app
Copy link

Sorry,@ZeroIntensity and@kumaraditya303, I could not cleanly backport this to3.12 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker de929f353c413459834a2a37b2d9b0240673d874 3.12

@bedevere-app
Copy link

GH-124573 is a backport of this pull request to the3.13 branch.

@bedevere-app
Copy link

GH-124574 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelSep 26, 2024
kumaraditya303 added a commit that referenced this pull requestSep 26, 2024
…pport e… (#124574)gh-124309: Modernize the `staggered_race` implementation to support eager task factories (#124390)Co-authored-by: Thomas Grainger <tagrain@gmail.com>Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>Co-authored-by: Carol Willing <carolcode@willingconsulting.com>Co-authored-by: Kumar Aditya <kumaraditya@python.org>(cherry picked from commitde929f3)Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@ZeroIntensityZeroIntensity deleted the fix-staggered-race-eager-task-factory branchSeptember 26, 2024 10:05
ZeroIntensity added a commit to ZeroIntensity/cpython that referenced this pull requestSep 30, 2024
Yhg1s pushed a commit that referenced this pull requestOct 1, 2024
…am (#124810)* Revert "GH-124639: add back loop param to staggered_race (#124700)"This reverts commite0a41a5.* Revert "gh-124309: Modernize the `staggered_race` implementation to support eager task factories (#124390)"This reverts commitde929f3.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestOct 1, 2024
…wnstream (pythonGH-124810)* Revert "pythonGH-124639: add back loop param to staggered_race (pythonGH-124700)"This reverts commite0a41a5.* Revert "pythongh-124309: Modernize the `staggered_race` implementation to support eager task factories (pythonGH-124390)"This reverts commitde929f3.(cherry picked from commit133e929)Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Yhg1s pushed a commit that referenced this pull requestOct 1, 2024
…ownstream (GH-124810) (#124817)gh-124309: Revert eager task factory fix to prevent breaking downstream (GH-124810)* Revert "GH-124639: add back loop param to staggered_race (GH-124700)"This reverts commite0a41a5.* Revert "gh-124309: Modernize the `staggered_race` implementation to support eager task factories (GH-124390)"This reverts commitde929f3.(cherry picked from commit133e929)Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra left review comments

@willingcwillingcwillingc left review comments

@graingertgraingertgraingert approved these changes

@kumaraditya303kumaraditya303kumaraditya303 approved these changes

@1st11st1Awaiting requested review from 1st11st1 is a code owner

@asvetlovasvetlovAwaiting requested review from asvetlovasvetlov is a code owner

@gvanrossumgvanrossumAwaiting requested review from gvanrossum

Assignees

@kumaraditya303kumaraditya303

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ZeroIntensity@graingert@JelleZijlstra@willingc@kumaraditya303

[8]ページ先頭

©2009-2025 Movatter.jp