Skip to content

bpo-33413: asyncio.gather without a special Future #6694

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

tecki
Copy link
Contributor

@tecki tecki commented May 2, 2018

This re-implements asyncio.gather without the need of a special inherited Future.

Futures nowadays are created using loop.create_future(), so having a special-case Future is actually weird. This changes gather() to simply return a Task.

This patch goes at great length to stay backwards compatible. It splits the function gather() in two parts: a backwards-compatible gather() that wraps the coroutine _gather() into an ensure_future(). IMHO the _gather() is everything one would need.

https://bugs.python.org/issue33413

tecki added 3 commits May 2, 2018 10:39
if a gather whose futures are already all done, but didn't get the
chance to return yet gets cancelled, it still has to propagate that
cancellation up the task chain.

In the past the test tests a proxy to that, some behavior of a
_GatheringFuture that will lead to this propagation. This test now tests
the real thing: we test that the cancellation is propagated.
in the past, a special _GatheringFuture inherited from Future.

This is very archaic. Now we simply use a Task.
gather now needs an event loop round more to work, just give it that
time.
@tecki tecki requested review from 1st1 and asvetlov as code owners May 2, 2018 18:45
@csabella csabella requested review from 1st1 and asvetlov and removed request for 1st1 and asvetlov October 18, 2019 01:14
@1st1
Copy link
Member

1st1 commented Oct 18, 2019

Ideally I'd like to not merge this is as this can be not fully backwards compatible. We'll likely have TaskGroups in asyncio 3.9 and will simply deprecate asyncio.gather.

That said, if @asvetlov and @tecki want to work on it I do actually appreciate the simplification of the code this PR leads to.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 17, 2022
@gvanrossum
Copy link
Member

@serhiy-storchaka Could you review this?

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 18, 2022
Copy link
Contributor

@kumaraditya303 kumaraditya303 left a comment

Choose a reason for hiding this comment

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

This has merge conflicts now.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@gvanrossum
Copy link
Member

@tecki are you still interested in pushing this forward? The PR is over 4 years old and the merge conflict seems pretty serious -- your best approach may be to start over from scratch.

@willingc
Copy link
Contributor

Hi @tecki, Thanks for opening this PR. I'm going to go ahead and close this PR. Should you wish to work on this issue in the future, please open a new PR. Thank you. ☀️

@willingc willingc closed this Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants