Skip to content
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

Deprecate bare connector close #3417

Merged
merged 3 commits into from
Dec 1, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Deprecate bare connector.close() call without awaiting
  • Loading branch information
asvetlov committed Dec 1, 2018
commit b9b40e028604ac03c341679547b556f5e8a2b05c
2 changes: 1 addition & 1 deletion aiohttp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 40 additions & 5 deletions aiohttp/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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}
Expand All @@ -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."""
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions aiohttp/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ def noop(*args, **kwargs): # type: ignore
return # type: ignore


async def noop2(*args: Any, **kwargs: Any) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename it to represent the intent:

Suggested change
async def noop2(*args: Any, **kwargs: Any) -> None:
async def async_noop(*args: Any, **kwargs: Any) -> None:

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Both noops are async.
  2. I don't care too much, the function is internal and will be removed in aiohttp 4.

return


coroutines._DEBUG = old_debug # type: ignore


Expand Down
15 changes: 12 additions & 3 deletions tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down