ClientSession created in aiohttp.request() remains unclosed

[das7pad]

Long story short

Using aiohttp.request() to file http requests spamms the logfiles.

The aiohttp.ClientSession created in aiohttp.request() should be closed gracefully, but currently the session reference is just deleted.

Expected behaviour

No resource warning from using aiohttp.request

Actual behaviour

Unclosed aiohttp.ClientSessions messages for every request
see tdryer/hangups#340

Steps to reproduce

call aiohttp.request
https://github.com/tdryer/hangups/blob/master/hangups/http_utils.py#L33-L36

From another project you can find a tester with results on different aiohttp version here:
https://gist.github.com/das7pad/e8f63175377576a0cb3466d20a417fa5

Your environment

python v3.5.3

aiohttp v2.2.0

[das7pad]

I already have a fix for this issue but as version 2.2.0 is not added to the v2 release branch, I can not submit a pull request to it that could pass all the checks then.
Using the main branch currently throws AssertionErrors for the refactored header handling in version 2.3.x and bad lower/upper case usage.

I created a branch from the version tag 2.2.0 and commited there: https://github.com/das7pad/aiohttp/tree/close-created-session
Feel free to cherrypick the commit das7pad@6052619 instead of a PR or guide me to which branch I should commit it to.

[asvetlov]

I recommend switch from aiohttp.request() to ClientSession.request() with proper session closing.

[das7pad]

But this change literally deprecates aiohttp.request().
Application X is now forced to have a more complex setup for a single http request with creating a session, then filing the request from this session and closing the request and session at the end. In addition it has to extend the Exception-handling to close the additional session resource.

In python version 3.5 one could simply use async with for the session and request, but for version 3.4 it is more than that:
https://gist.github.com/das7pad/4ff20e6a8e7e3242bffb9ee9c30809d3
Every application has to change the http requests now when using aiohttp.request before.

[fafhrd91]

we can silence warning for aiohttp.request()

[asvetlov]

we can silence warning for aiohttp.request()

Good to me.

Honestly I want to remove aiohttp.request() at all. Maybe along with future drop of Python 3.4 support.
Is there reason to keep the function?

[fafhrd91]

simplicity.
@asvetlov I noticed, you like ceremony a lot :)

[asvetlov]

aiohttp is OSS project.
Our changes and overall design should be clean and visible for library users.
It's not a ceremony for ceremony but a way to reduce breaking changes.

Well, I could buy your argument for keeping aiohttp.request(). Forcing usage to async with aiohttp.request() could get a place for proper session shutdown.

[fafhrd91]

What could be cleaner than aiohttp.request() ?
Even with async with you can get same warning, even with with ClientSession you can get warning. Python just can't enforce right usage. Did you forget your change to clientresponse? Closing session after exit from context is not safe operation.

[asvetlov]

Ok, let's just suppress a warning

[das7pad]

Closing session after exit from context is not safe operation.

@fafhrd91 is that a note to this line? https://gist.github.com/das7pad/4ff20e6a8e7e3242bffb9ee9c30809d3#file-aiohttp_fetch_comparison-py-L29

[fafhrd91]

@das7pad I am talking about using session as async context manager. you example is fine.

[asvetlov]

The functionality was changed and fixed in aiohttp 3.0

[lock]

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].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new