Deprecate bare connector close (#3417)

CHANGES/3417.removal

@@ -0,0 +1 @@
+Deprecate bare connector close, use ``async with connector:`` and ``await connector.close()`` instead.

aiohttp/client.py

@@ -819,7 +819,7 @@ async def close(self) -> None:
         """
         if not self.closed:
             if self._connector is not None and self._connector_owner:
-                self._connector.close()
+                await self._connector.close()
             self._connector = None
 
     @property

aiohttp/connector.py

@@ -9,6 +9,7 @@
 from http.cookies import SimpleCookie
 from itertools import cycle, islice
 from time import monotonic
+from types import TracebackType
 from typing import (TYPE_CHECKING, Any, Awaitable, Callable,  # noqa
                     DefaultDict, Dict, Iterator, List, Optional, Set, Tuple,
                     Type, Union, cast)
@@ -27,7 +28,7 @@
 from .client_proto import ResponseHandler
 from .client_reqrep import ClientRequest, Fingerprint, _merge_ssl_params
 from .helpers import (PY_36, CeilTimeout, get_running_loop, is_ip_address,
-                      noop, sentinel)
+                      noop2, sentinel)
 from .http import RESPONSES
 from .locks import EventResultOrError
 from .resolver import DefaultResolver
@@ -50,6 +51,24 @@
     from .tracing import Trace  # noqa
 
 
+class _DeprecationWaiter:
+    __slots__ = ('_awaitable', '_awaited')
+
+    def __init__(self, awaitable: Awaitable[Any]) -> None:
+        self._awaitable = awaitable
+        self._awaited = False
+
+    def __await__(self) -> Any:
+        self._awaited = True
+        return self._awaitable.__await__()
+
+    def __del__(self) -> None:
+        if not self._awaited:
+            warnings.warn("Connector.close() is a coroutine, "
+                          "please use await connector.close()",
+                          DeprecationWarning)
+
+
 class Connection:
 
     _source_traceback = None
@@ -223,7 +242,7 @@ def __del__(self, _warnings: Any=warnings) -> None:
 
         conns = [repr(c) for c in self._conns.values()]
 
-        self.close()
+        self._close()
 
         if PY_36:
             kwargs = {'source': self}
@@ -240,11 +259,24 @@ def __del__(self, _warnings: Any=warnings) -> None:
         self._loop.call_exception_handler(context)
 
     def __enter__(self) -> 'BaseConnector':
+        warnings.warn('"witn Connector():" is deprecated, '
+                      'use "async with Connector():" instead',
+                      DeprecationWarning)
         return self
 
     def __exit__(self, *exc: Any) -> None:
         self.close()
 
+    async def __aenter__(self) -> 'BaseConnector':
+        return self
+
+    async def __aexit__(self,
+                        exc_type: Optional[Type[BaseException]]=None,
+                        exc_value: Optional[BaseException]=None,
+                        exc_traceback: Optional[TracebackType]=None
+                        ) -> None:
+        await self.close()
+
     @property
     def force_close(self) -> bool:
         """Ultimately close connection on releasing if True."""
@@ -334,14 +366,18 @@ def _cleanup_closed(self) -> None:
 
     def close(self) -> Awaitable[None]:
         """Close all opened transports."""
+        self._close()
+        return _DeprecationWaiter(noop2())
+
+    def _close(self) -> None:
         if self._closed:
-            return noop()
+            return
 
         self._closed = True
 
         try:
             if self._loop.is_closed():
-                return noop()
+                return
 
             # cancel cleanup task
             if self._cleanup_handle:
@@ -369,7 +405,6 @@ def close(self) -> Awaitable[None]:
             self._cleanup_handle = None
             self._cleanup_closed_transports.clear()
             self._cleanup_closed_handle = None
-        return noop()
 
     @property
     def closed(self) -> bool:

aiohttp/helpers.py

@@ -82,6 +82,10 @@ def noop(*args, **kwargs):  # type: ignore
     return  # type: ignore
 
 
+async def noop2(*args: Any, **kwargs: Any) -> None:
+    return
+
+
 coroutines._DEBUG = old_debug  # type: ignore
 
 

docs/client_reference.rst

@@ -843,7 +843,7 @@ BaseConnector
 
       Read-only property.
 
-   .. method:: close()
+   .. comethod:: close()
 
       Close all opened connections.
 

tests/test_connector.py

@@ -222,12 +222,21 @@ async def test_create_conn(loop) -> None:
 
 async def test_context_manager(loop) -> None:
     conn = aiohttp.BaseConnector(loop=loop)
-    conn.close = mock.Mock()
 
-    with conn as c:
+    with pytest.warns(DeprecationWarning):
+        with conn as c:
+            assert conn is c
+
+    assert conn.closed
+
+
+async def test_async_context_manager(loop) -> None:
+    conn = aiohttp.BaseConnector(loop=loop)
+
+    async with conn as c:
         assert conn is c
 
-    assert conn.close.called
+    assert conn.closed
 
 
 async def test_close(loop) -> None: