Skip to content

Conversation

asvetlov
Copy link
Member

@asvetlov asvetlov commented Dec 1, 2018

Use async with connector: and await connector.close() instead

@asvetlov asvetlov requested a review from webknjaz as a code owner December 1, 2018 14:11
@codecov-io
Copy link

codecov-io commented Dec 1, 2018

Codecov Report

Merging #3417 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3417      +/-   ##
==========================================
+ Coverage   97.94%   97.95%   +<.01%     
==========================================
  Files          44       44              
  Lines        8522     8543      +21     
  Branches     1383     1383              
==========================================
+ Hits         8347     8368      +21     
  Misses         74       74              
  Partials      101      101
Impacted Files Coverage Δ
aiohttp/helpers.py 97.2% <100%> (+0.01%) ⬆️
aiohttp/connector.py 97.67% <100%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0764c5...a2ea6ef. Read the comment docs.

@@ -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.

@asvetlov asvetlov merged commit 1618fe6 into master Dec 1, 2018
@asvetlov asvetlov deleted the deprecate-bare-connector-close branch December 1, 2018 23:06
@atemate atemate mentioned this pull request Jun 28, 2019
5 tasks
@lock
Copy link

lock bot commented Dec 2, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Dec 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 2, 2019
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Dec 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bot:chronographer:provided There is a change note present in this PR outdated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants