bpo-32193: Convert asyncio to async/await usage #4753

asvetlov · 2017-12-07T22:13:19Z

The PR is not finished yet but subprocesses, streams and queues are converted.

https://bugs.python.org/issue32193

asvetlov · 2017-12-07T22:14:35Z

Lib/asyncio/locks.py

@@ -71,15 +71,13 @@ def __await__(self):
        yield from self.acquire()


with await lock prevents converting .acquire() to async function. This is sad.

acquire can be converted to async def, just do this:

async def acquire(self): ... def __await__(self): return self.acquire().__await__()

I'd also raise a DeprecationWarning in Lock.__await__. The new idiomatic way is to write async with lock. We can open a separate issue for this.

The problem is that __await__(self) should return not awaitable for self.acquire() but an awaitable object with __enter__ / __exit__ for usage in statements like:

lock = asyncio.Lock() with await lock as _lock: assert _lock is None assert lock.locked()

Alright, you can do this then:

async def __acquire_ctx(self): await self.acquire() return _ContextManager(self) def __await__(self): return self.__acquire_ctx().__await__()

(use two leading underscores, I want 0 chance of someone using acquire_ctx method)

asvetlov · 2017-12-07T22:17:50Z

asyncio.sleep() uses bare yield for zero timeout.
I can convert it to loop.call_soon(...) if we agree to use this approach.

1st1 · 2017-12-07T22:28:24Z

I can convert it to loop.call_soon(...) if we agree to use this approach.

sleep(0) is frequently used to skip a beat of the event loop... I'd like it to be as fast as possible. Can you check if this works:

@types.coroutine
def __sleep0():
    yield

async def sleep(...):
    if delay == 0:
        await __sleep0()
        return result
    ...

asvetlov · 2017-12-07T22:29:18Z

Another question: should stubs from AbstractEventLoop be converted to async functions?
Now they are just regular funcs not decorated by @coroutine, e.g. def connect_read_pipe(self, protocol_factory, pipe): ...
I believe changing the signature to async def connect_read_pipe(self, protocol_factory, pipe): ... is backward compatible (tests should be fixed but semantics is kept the same) but more obvious.

1st1 · 2017-12-07T22:36:03Z

Another question: should stubs from AbstractEventLoop be converted to async functions?

Don't see any problem with converting them.

asvetlov · 2017-12-07T23:37:25Z

asyncio.sleep is done

1st1 · 2017-12-07T23:43:08Z

Lib/asyncio/tasks.py

-def sleep(delay, result=None, *, loop=None):
+@types.coroutine
+def _sleep0():
+    yield


Need a comment here explaining why we have this hack.

asvetlov · 2017-12-08T10:44:23Z

Stubs are converted

asvetlov · 2017-12-08T16:49:09Z

I have made the requested changes; please review again

bedevere-bot · 2017-12-08T16:49:11Z

Thanks for making the requested changes!

@1st1: please review the changes made to this pull request.

1st1 · 2017-12-08T17:28:48Z

Lib/test/test_asyncio/test_tasks.py

+
+        @asyncio.coroutine
+        def coro():
+            return 'ok'


Please test both types of old-style coroutines in this test: the ones that have 'yield from' in them and the ones that don't:

@coroutine def coro1(): return 'ok' @coroutine def coro2(): yield from asyncio.sleep(0) return 'ok'

1st1 · 2017-12-08T17:29:39Z

Lib/test/test_asyncio/test_windows_events.py

@@ -56,6 +56,7 @@ def test_pipe(self):
        res = self.loop.run_until_complete(self._test_pipe())
        self.assertEqual(res, 'done')

+    @asyncio.coroutine


Shouldn't this be converted to 'async def' too?

I was trying to minimize tests change for sake of easy review.
One line for adding missing @asyncio.coroutine or several yield from -> await replacements.

I think it's OK to upgrade tests too.

asvetlov · 2017-12-08T17:37:00Z

Everything is converted.

Should Doc/whatsnew/3.7.rst be updated?

1st1 · 2017-12-08T17:39:56Z

Should Doc/whatsnew/3.7.rst be updated?

Up to you. Elvis will take care of it anyways.

1st1 · 2017-12-08T17:40:53Z

Also, can you apply your patch and run aiohttp tests with the updated asyncio codebase?

1st1 · 2017-12-08T18:17:58Z

Lib/asyncio/tasks.py

+    # the hack is needed to make a quick pass
+    # of task step iteration when delay == 0
+    # Task._step has an optimization for bare yield
+    yield


Please rewrite to:

"""Skip one event loop run cycle. This is a private helper for 'asyncio.sleep()', used when the 'delay' is set to 0. It uses a bare 'yield' expression (which Task._step knows how to handle) instead of creating a Future object. """

asvetlov · 2017-12-08T21:52:01Z

Comments are addressed.
aiohttp works perfectly fine.

1st1

OK, let's merge it! Thanks, Andrew.

asvetlov · 2017-12-08T22:24:01Z

Thanks for careful review

1st1 · 2017-12-08T22:27:17Z

BTW, can you write a test that with (yield from lock) is still working?

asvetlov · 2017-12-09T08:44:21Z

Let's do it as part of https://bugs.python.org/issue32253

berkerpeksag · 2017-12-09T09:26:34Z

@asvetlov could you please strip commit messages like "Code cleanup" and "Fix indentation" next time when you merge a PR? See the step 3 at https://devguide.python.org/gitbootcamp/#accepting-and-merging-a-pull-request for details.

asvetlov · 2017-12-09T09:37:54Z

@berkerpeksag will do.

asvetlov added 8 commits December 7, 2017 23:21

Convert asyncio/tasks.py to async/await

db463e3

Convert asyncio/queues.py to async/await

bdde875

Convert asyncio/test_utils.py to async/await

a44c892

Convert asyncio/base_subprocess.py to async/await

ee5ecc7

Convert asyncio/subprocess.py to async/await

5654dcf

Convert asyncio/streams.py to async/await

0038376

Fix comments

3e90af4

Convert asyncio/locks.py to async/await

ca9c418

asvetlov requested review from 1st1, gpshead and rhettinger as code owners December 7, 2017 22:13

the-knights-who-say-ni added the CLA signed label Dec 7, 2017

bedevere-bot added the awaiting merge label Dec 7, 2017

asvetlov commented Dec 7, 2017

View reviewed changes

1st1 removed request for gpshead and rhettinger December 7, 2017 22:16

Convert asyncio.sleep to async def

c0ec04e

1st1 reviewed Dec 7, 2017

View reviewed changes

asvetlov added 3 commits December 8, 2017 07:38

Add a comment

f6d230d

Add missing news

251d990

Convert stubs from AbstrctEventLoop to async functions

983552a

asvetlov added 3 commits December 8, 2017 13:14

Convert subprocess_shell/subprocess_exec

618fcee

Convert connect_read_pipe/connect_write_pip to async/await syntax

cfab76e

Convert create_datagram_endpoint

ee8c389

bedevere-bot added awaiting change review and removed awaiting changes labels Dec 8, 2017

1st1 reviewed Dec 8, 2017

View reviewed changes

Convert asyncio/locks.py

7c80c7c

1st1 reviewed Dec 8, 2017

View reviewed changes

asvetlov added 7 commits December 8, 2017 22:25

Improve docstring

1c0c2e5

Convert tests to async/await

d1a9a96

Convert more tests

17a5969

Convert more tests

f159ca7

Convert more tests

74cd8f3

Convert tests

19460e5

Improve test

61cf3eb

1st1 approved these changes Dec 8, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Dec 8, 2017

asvetlov merged commit 5f841b5 into python:master Dec 8, 2017

bedevere-bot removed the awaiting merge label Dec 8, 2017

asvetlov deleted the async-await branch December 8, 2017 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-32193: Convert asyncio to async/await usage #4753

bpo-32193: Convert asyncio to async/await usage #4753

asvetlov commented Dec 7, 2017 •

edited by bedevere-bot

Loading

asvetlov Dec 7, 2017

1st1 Dec 7, 2017

asvetlov Dec 8, 2017

1st1 Dec 8, 2017 •

edited

Loading

1st1 Dec 8, 2017

asvetlov Dec 8, 2017

asvetlov commented Dec 7, 2017

1st1 commented Dec 7, 2017 •

edited

Loading

asvetlov commented Dec 7, 2017

1st1 commented Dec 7, 2017

asvetlov commented Dec 7, 2017

1st1 Dec 7, 2017

asvetlov Dec 8, 2017

asvetlov commented Dec 8, 2017

asvetlov commented Dec 8, 2017

bedevere-bot commented Dec 8, 2017

1st1 Dec 8, 2017

1st1 Dec 8, 2017

asvetlov Dec 8, 2017

1st1 Dec 8, 2017

asvetlov commented Dec 8, 2017

1st1 commented Dec 8, 2017

1st1 commented Dec 8, 2017

1st1 Dec 8, 2017

asvetlov commented Dec 8, 2017

1st1 left a comment

asvetlov commented Dec 8, 2017

1st1 commented Dec 8, 2017

asvetlov commented Dec 9, 2017

berkerpeksag commented Dec 9, 2017

asvetlov commented Dec 9, 2017

		@@ -71,15 +71,13 @@ def __await__(self):
		yield from self.acquire()

bpo-32193: Convert asyncio to async/await usage #4753

bpo-32193: Convert asyncio to async/await usage #4753

Conversation

asvetlov commented Dec 7, 2017 • edited by bedevere-bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1st1 Dec 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov commented Dec 7, 2017

1st1 commented Dec 7, 2017 • edited Loading

asvetlov commented Dec 7, 2017

1st1 commented Dec 7, 2017

asvetlov commented Dec 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov commented Dec 8, 2017

asvetlov commented Dec 8, 2017

bedevere-bot commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov commented Dec 8, 2017

1st1 commented Dec 8, 2017

1st1 commented Dec 8, 2017

Choose a reason for hiding this comment

asvetlov commented Dec 8, 2017

1st1 left a comment

Choose a reason for hiding this comment

asvetlov commented Dec 8, 2017

1st1 commented Dec 8, 2017

asvetlov commented Dec 9, 2017

berkerpeksag commented Dec 9, 2017

asvetlov commented Dec 9, 2017

asvetlov commented Dec 7, 2017 •

edited by bedevere-bot

Loading

1st1 Dec 8, 2017 •

edited

Loading

1st1 commented Dec 7, 2017 •

edited

Loading