bpo-32253: Deprecate with statement and bare await for asyncio locks #4764

asvetlov · 2017-12-09T09:14:31Z

https://bugs.python.org/issue32253

1st1 · 2017-12-09T16:12:33Z

Doc/library/asyncio-sync.rst


   This class is :ref:`not thread safe <asyncio-multithreading>`.

-   Usage::


~~Let's have one usage example with 'async with ...' syntax.~~

Never mind, I see that you added a new section for that...

1st1 · 2017-12-09T16:13:59Z

Doc/library/asyncio-sync.rst

+------------------------------------------------------------------------------
+
+All of the objects provided by this module that have :meth:`acquire`
+and :meth:`release` methods can be used as context managers for a


"can be used as context managers for a" -> "can be used in "async with" statements"

1st1 · 2017-12-09T16:15:30Z

Doc/library/asyncio-sync.rst

+
+Currently, :class:`Lock`, :class:`Condition`,
+:class:`Semaphore`, and :class:`BoundedSemaphore` objects may be used as
+:keyword:`async with` statement context managers.


This is a bit repetitive... I'd merge this paragraph with the first one:

":class:Lock, :class:Condition, :class:Semaphore, and :class:BoundedSemaphore can be used in "async with" statements."

1st1 · 2017-12-09T16:16:55Z

Doc/library/asyncio-sync.rst

+synchronization primitives.
+
+Explicit :meth:`acquire` / :meth:`release` calls should be used if
+locking/unlocking is split into different functions.


I don't think we need these two paragraphs. People usually naturally prefer 'with' blocks when they are available.

1st1 · 2017-12-09T16:17:26Z

Doc/library/asyncio-sync.rst

+
+.. deprecated:: 3.7
+
+   Lock acquiring on ``await lock`` or ``yield from lock`` and


"Lock acquiring on" -> "Lock acquiring using"

asvetlov · 2017-12-09T17:16:21Z

All notes are fixed

1st1 · 2017-12-09T17:21:12Z

Doc/library/asyncio-sync.rst

@@ -260,6 +232,8 @@ Semaphore
   defaults to ``1``. If the value given is less than ``0``, :exc:`ValueError`
   is raised.

+   Locks also support the :ref:`context management protocol <async-with-locks>`.


Semaphores also support...

1st1 · 2017-12-09T17:21:22Z

Doc/library/asyncio-sync.rst


 .. class:: BoundedSemaphore(value=1, \*, loop=None)

   A bounded semaphore implementation. Inherit from :class:`Semaphore`.

   This raises :exc:`ValueError` in :meth:`~Semaphore.release` if it would
   increase the value above the initial value.
+
+   Locks also support the :ref:`context management protocol <async-with-locks>`.


BoundedSemaphore also support...

1st1 · 2017-12-09T17:22:33Z

Spotted a few more nits; other than that it looks good

asvetlov · 2017-12-09T17:25:00Z

Fixed

1st1 · 2017-12-09T17:25:52Z

Doc/library/asyncio-sync.rst

@@ -139,7 +139,8 @@ Condition
   object, and it is used as the underlying lock.  Otherwise,
   a new :class:`Lock` object is created and used as the underlying lock.

-   Locks also support the :ref:`context management protocol <async-with-locks>`.
+   Conditions also support the :ref:`context management protocol


Last nit -- drop "also"

1st1

lgtm. Thanks!

asvetlov added 3 commits Dec 9, 2017

Add test for with (yield from lock)

5855aa5

Deprecate with statement for asyncio locks

ca77820

Document deprecation of with statement for asyncio locks

db2167b

asvetlov requested a review from 1st1 as a code owner Dec 9, 2017

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

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

asvetlov added 2 commits Dec 9, 2017

Add NEWS entry

04e1b8e

Simplify docs structure for asyncio locks

694a769

1st1 reviewed Dec 9, 2017

View changes

asvetlov added 2 commits Dec 9, 2017

Fix notes

3159fca

Fix test

c57bccb

Drop redundant lines

a069dd6

1st1 reviewed Dec 9, 2017

View changes

Fix text

2aeb296

1st1 reviewed Dec 9, 2017

View changes

Fix notes

124a305

1st1 approved these changes Dec 9, 2017

View changes

asvetlov merged commit 28d8d14 into python:master Dec 9, 2017

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

asvetlov deleted the bpo-32253 branch Dec 9, 2017

bpo-32253: Deprecate with statement and bare await for asyncio locks #4764

bpo-32253: Deprecate with statement and bare await for asyncio locks #4764

asvetlov commented Dec 9, 2017 •

edited by bedevere-bot

1st1 Dec 9, 2017 •

edited

1st1 Dec 9, 2017

1st1 Dec 9, 2017

1st1 Dec 9, 2017

1st1 Dec 9, 2017

1st1 Dec 9, 2017

asvetlov commented Dec 9, 2017

1st1 Dec 9, 2017

asvetlov Dec 9, 2017

1st1 Dec 9, 2017

asvetlov Dec 9, 2017

1st1 commented Dec 9, 2017

asvetlov commented Dec 9, 2017

1st1 Dec 9, 2017

asvetlov Dec 9, 2017

1st1 left a comment


		This class is :ref:`not thread safe <asyncio-multithreading>`.

		Usage::


		.. deprecated:: 3.7

		Lock acquiring on ``await lock`` or ``yield from lock`` and

bpo-32253: Deprecate with statement and bare await for asyncio locks #4764

bpo-32253: Deprecate with statement and bare await for asyncio locks #4764

Conversation

asvetlov commented Dec 9, 2017 • edited by bedevere-bot

1st1 Dec 9, 2017 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov commented Dec 9, 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

1st1 commented Dec 9, 2017

asvetlov commented Dec 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1st1 left a comment

asvetlov commented Dec 9, 2017 •

edited by bedevere-bot

1st1 Dec 9, 2017 •

edited