gh-95601: restore support for awaitable objects that are not futures in `asyncio.wait` #95708

graingert · 2022-08-05T11:13:30Z

Issue: support for Awaitable objects in asyncio.wait removed in 3.11 #95601

Issue: asyncio.wait accepts concurrent.futures.Future objects and other more exotic objects like distributed.Future #95920

Doc/library/asyncio-task.rst

kumaraditya303 · 2022-08-09T13:10:52Z

Lib/asyncio/tasks.py

+        if coroutines.iscoroutine(f):
+            raise TypeError("Passing coroutines is forbidden, use tasks explicitly.")
+        if not futures.isfuture(f):
+            warnings.warn(


Use warnings._deprecated?

Yeah I usually would, but I kinda want to try backport the deprecation fix to 3.10

also the wording warnings._deprecated doesn't work too well for changes in behaviour and I didn't want to try and fix it and open a new can of worms there

@hugovk are you already working on a plan to upbraid all the existing warnings.warn(..., DeprecationWarning) to warnings._deprecated ?

there's a slight usability/wording concern when using it for changes in behavior instead of actual full removals
and there also seems to be a desire to use it for totally unscheduled removals: remove=(4, )

in addition do you have a view on backporting deprecation, where it was clear from the code what the intention of the deprecation was, but the deprecation itself had a bug?

@hugovk are you already working on a plan to upbraid all the existing warnings.warn(..., DeprecationWarning) to warnings._deprecated ?

I'm not but that's a good idea. From a quick check, there's not too many, and most are set to be removed in 3.12 anyway.

there's a slight usability/wording concern when using it for changes in behavior instead of actual full removals

Can you alter the wording using the message parameter?

and there also seems to be a desire to use it for totally unscheduled removals: remove=(4, )

Some removals scheduled for 4.0 were from before we knew if 3.10 or 4.0 would follow 3.9 (#80480 (comment)) and in at least one case is shorthand for "sometime after 2.7.final" (#80480 (comment)).

Anyway, we should review any remaining removals originally set for 4.0 (e.g. in new issues or relevant open issues) to decide if they can be removed in an upcoming 3.x.

in addition do you have a view on backporting deprecation, where it was clear from the code what the intention of the deprecation was, but the deprecation itself had a bug?

Generally, I think it's a good idea to increase visibility and help users upgrade in time to reduce friction. It's good to ask release managers when in doubt.

@hugovk do you think we have enough here to make a new issue out of this side discussion. Or maybe there's two to four ideas here:

Do something about existing deprecation warnings that don't automatically get upgraded to errors

Do something about the wording and format-string templating in warnings._deprecated to make it work for even the most subtle behaviour change

Determine a backport strategy for when a deprecation warning was applied to the wrong thing

Determine a strategy for permanent deprecations, where a feature is a bit broken or an alias of another thing, and people should really use the not broken way or source of the alias, but CPython is unwilling or undecided on actually removing it

Sure! Or at https://discuss.python.org/?

Lib/test/test_asyncio/test_tasks.py

Misc/NEWS.d/next/Library/2022-08-05-11-16-33.gh-issue-95601.hJvI9y.rst

Lib/test/test_asyncio/test_tasks.py

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

…vI9y.rst Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Lib/test/test_asyncio/test_tasks.py

graingert · 2022-09-12T07:05:13Z

FWIW I don't think this should be deferred blocker. cc @pablogsal

If is adding a deprecation, is a blocker because we certainly won't be able to do it after the first release. It still needs an exemption though

this is undoing a removal that didn't go via the deprecation policy, and then applying a deprecation for restored functionality

kumaraditya303

LGTM

gvanrossum · 2022-09-12T18:31:23Z

Please don't add release-blocker labels without talking to Pablo first.

graingert · 2022-09-13T06:04:44Z

@tiran recommended spitting this PR into two, one that restores the functionality removed without deprecation and one that does the deprecation.

Misc/NEWS.d/next/Library/2022-08-05-11-16-33.gh-issue-95601.hJvI9y.rst

…vI9y.rst

serhiy-storchaka · 2022-09-14T07:39:04Z

It seems that the function was always supposed to only support futures and tasks. If you want to add support of arbitrary awaitables, it should be a new feature. And you need to update the documentation, the docstring and error messages to reflect this.

graingert · 2022-09-14T07:40:43Z

It seems that the function was always supposed to only support futures and tasks. If you want to add support of arbitrary awaitables, it should be a new feature. And you need to update the documentation, the docstring and error messages to reflect this.

that's correct however support for Awaitables is currently available in 3.7, 3.8, 3.9, 3.10 and was removed as a consequence of an incorrectly enforced deprecation in 3.11

I plan to deprecate this support correctly in #96783

serhiy-storchaka · 2022-09-14T07:46:07Z

Lib/asyncio/tasks.py

+
+    for f in fs:
+        if coroutines.iscoroutine(f):
+            raise TypeError("Passing coroutines is forbidden, use tasks explicitly.")


What will happened with implicitly created futures added to new_fs? Would it emit a warning?

I removed the warning at @tiran 's request, with a plan to add it in a follow up PR.
I'm happy to put it back in this PR if you'd prefer

graingert · 2022-09-28T12:03:44Z

@serhiy-storchaka can you re-review this? I'll add the warning if you and @tiran agree it should be added in this PR.

gvanrossum

I discussed this with @1st1 and a few others at the core dev sprint today, and we're not excited about this. There's definitely no support for trying to wedge this into the 3.11 release at this point.

IIRC @1st1 is planning to add his own response.

kumaraditya303 · 2022-10-27T17:52:58Z

My previous reviews should be discarded as the code has changed since I reviewed. (I don't think I have the permission to dismiss stale reviews.). Since 3.11 is final now, it can't be backported either. I am hoping that @gvanrossum and @1st1 can handle this for 3.12.

gvanrossum · 2022-10-27T17:54:38Z

I think it’s better to start over with a clean slate.

pythongh-95601: forbid non-futures in asyncio.wait

88580c9

bedevere-bot added the awaiting review label Aug 5, 2022

blurb-it bot and others added 3 commits August 5, 2022 11:16

📜🤖 Added by blurb_it.

1363c9b

Merge branch 'main' into forbid-non-futures-in-asyncio-wait

543c4f2

clarify awaitable-not-future

6661052

graingert changed the title ~~gh-95601: forbid non-futures in asyncio.wait~~ gh-95601: restore support for awaitable objects that are not futures in asyncio.wait and deprecate that support Aug 5, 2022

graingert changed the title ~~gh-95601: restore support for awaitable objects that are not futures in asyncio.wait and deprecate that support~~ gh-95601: restore support for awaitable objects that are not futures in asyncio.wait and deprecate that support Aug 5, 2022

update asyncio.wait deprecation notice

5b740df

graingert commented Aug 5, 2022

View reviewed changes

Doc/library/asyncio-task.rst Outdated Show resolved Hide resolved

Update Doc/library/asyncio-task.rst

5b974c5

graingert marked this pull request as ready for review August 5, 2022 11:52

graingert requested review from 1st1 and asvetlov as code owners August 5, 2022 11:52

graingert added 2 commits August 5, 2022 12:52

Merge branch 'main' into forbid-non-futures-in-asyncio-wait

c4a7237

Merge branch 'main' into forbid-non-futures-in-asyncio-wait

d4dac27

graingert requested a review from kumaraditya303 August 9, 2022 11:11

kumaraditya303 requested changes Aug 9, 2022

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Aug 9, 2022

kumaraditya303 added needs backport to 3.11 only security fixes type-bug An unexpected behavior, bug, or error labels Aug 9, 2022

kumaraditya303 reviewed Aug 9, 2022

View reviewed changes

Lib/test/test_asyncio/test_tasks.py Outdated Show resolved Hide resolved

kumaraditya303 reviewed Aug 9, 2022

View reviewed changes

Lib/test/test_asyncio/test_tasks.py Outdated Show resolved Hide resolved

kumaraditya303 reviewed Aug 9, 2022

View reviewed changes

Lib/test/test_asyncio/test_tasks.py Outdated Show resolved Hide resolved

graingert and others added 4 commits August 9, 2022 15:18

Apply suggestions from code review

cb00279

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Update Misc/NEWS.d/next/Library/2022-08-05-11-16-33.gh-issue-95601.hJ…

38fb812

…vI9y.rst Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

Update Lib/test/test_asyncio/test_tasks.py

5383116

upgrade WaitTests to an IsolatedAsyncioTestCase

b6ea455

graingert commented Aug 9, 2022

View reviewed changes

Lib/test/test_asyncio/test_tasks.py Outdated Show resolved Hide resolved

Update Lib/test/test_asyncio/test_tasks.py

980a3b9

Merge branch 'main' into forbid-non-futures-in-asyncio-wait

9189603

graingert requested a review from gvanrossum as a code owner September 12, 2022 07:05

graingert requested a review from kumaraditya303 September 12, 2022 16:17

kumaraditya303 added release-blocker and removed deferred-blocker labels Sep 12, 2022

kumaraditya303 approved these changes Sep 12, 2022

View reviewed changes

gvanrossum removed the release-blocker label Sep 12, 2022

graingert added 3 commits September 12, 2022 19:47

don't deprecate Awaitables in wait yet

9c325b4

Merge branch 'main' into forbid-non-futures-in-asyncio-wait

da9f2a2

remove deprecation docs

7e35d9c

graingert changed the title ~~gh-95601: restore support for awaitable objects that are not futures in asyncio.wait and deprecate that support~~ gh-95601: restore support for awaitable objects that are not futures in asyncio.wait Sep 13, 2022

graingert mentioned this pull request Sep 13, 2022

deprecate support for awaitable objects that are not futures in asyncio.wait #96783

Closed

remove deprecation assertions from tests for asyncio.wait

a5c5f4a

graingert force-pushed the forbid-non-futures-in-asyncio-wait branch from a4c6c1c to a5c5f4a Compare September 13, 2022 06:13

graingert commented Sep 13, 2022

View reviewed changes

Misc/NEWS.d/next/Library/2022-08-05-11-16-33.gh-issue-95601.hJvI9y.rst Outdated Show resolved Hide resolved

Update Misc/NEWS.d/next/Library/2022-08-05-11-16-33.gh-issue-95601.hJ…

17cbfc9

…vI9y.rst

graingert requested a review from pablogsal September 13, 2022 06:57

Merge branch 'main' into forbid-non-futures-in-asyncio-wait

69b2a0d

serhiy-storchaka reviewed Sep 14, 2022

View reviewed changes

graingert requested a review from serhiy-storchaka September 15, 2022 06:58

gvanrossum reviewed Oct 6, 2022

View reviewed changes

gvanrossum closed this Oct 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-95601: restore support for awaitable objects that are not futures in `asyncio.wait` #95708

gh-95601: restore support for awaitable objects that are not futures in `asyncio.wait` #95708

graingert commented Aug 5, 2022 •

edited

Loading

kumaraditya303 Aug 9, 2022

graingert Aug 9, 2022 •

edited

Loading

graingert Aug 11, 2022

hugovk Aug 15, 2022

graingert Aug 16, 2022

hugovk Aug 17, 2022

graingert commented Sep 12, 2022

kumaraditya303 left a comment

gvanrossum commented Sep 12, 2022

graingert commented Sep 13, 2022

serhiy-storchaka commented Sep 14, 2022

graingert commented Sep 14, 2022 •

edited

Loading

serhiy-storchaka Sep 14, 2022

graingert Sep 14, 2022 •

edited

Loading

graingert commented Sep 28, 2022

gvanrossum left a comment

kumaraditya303 commented Oct 27, 2022

gvanrossum commented Oct 27, 2022

gh-95601: restore support for awaitable objects that are not futures in asyncio.wait #95708

gh-95601: restore support for awaitable objects that are not futures in asyncio.wait #95708

Conversation

graingert commented Aug 5, 2022 • edited Loading

kumaraditya303 Aug 9, 2022

Choose a reason for hiding this comment

graingert Aug 9, 2022 • edited Loading

Choose a reason for hiding this comment

graingert Aug 11, 2022

Choose a reason for hiding this comment

hugovk Aug 15, 2022

Choose a reason for hiding this comment

graingert Aug 16, 2022

Choose a reason for hiding this comment

hugovk Aug 17, 2022

Choose a reason for hiding this comment

graingert commented Sep 12, 2022

kumaraditya303 left a comment

Choose a reason for hiding this comment

gvanrossum commented Sep 12, 2022

graingert commented Sep 13, 2022

serhiy-storchaka commented Sep 14, 2022

graingert commented Sep 14, 2022 • edited Loading

serhiy-storchaka Sep 14, 2022

Choose a reason for hiding this comment

graingert Sep 14, 2022 • edited Loading

Choose a reason for hiding this comment

graingert commented Sep 28, 2022

gvanrossum left a comment

Choose a reason for hiding this comment

kumaraditya303 commented Oct 27, 2022

gvanrossum commented Oct 27, 2022

gh-95601: restore support for awaitable objects that are not futures in `asyncio.wait` #95708

gh-95601: restore support for awaitable objects that are not futures in `asyncio.wait` #95708

graingert commented Aug 5, 2022 •

edited

Loading

graingert Aug 9, 2022 •

edited

Loading

graingert commented Sep 14, 2022 •

edited

Loading

graingert Sep 14, 2022 •

edited

Loading