Encourage creation of aiohttp public objects inside a coroutine

[asvetlov]

I would enforce it. For example, constructing CleintSession in a module global namespace is a bad smell and leads to hard-to-debug errors very often.

The plan is:

1. Add helpers.get_running_loop() function with the following implementation:

def get_running_loop(loop=None):
    if loop is None:
        loop = asyncio.get_event_loop()
    if not loop.is_running():
        warnings.warn("The object should be created from async function",
                                 DeprecationWarning,
                                 stacklevel=3)

2. Replace asyncio.get_event_loop() with get_running_loop() in all aiohttp objects.
3. Replace a deprecation warning with RuntimeError() in aiohttp 4.0

[aio-libs-bot]

GitMate.io thinks possibly related issues are #5 (ServerHttpProtocol: automate Response object creation), #2224 (aiohttp.TCPConnector()), #2320 (aiohttp-jinja2), #2505 (Create a TestClient object inside a unittest_run_loop decorated AioHTTPTestCase method), and #58 (aiohttp.HttpClient).

[gjcarneiro]

I was worried about this, but I think the title is slightly misleading. I think it's ok to create it as long as a coroutine is part of the call stack, right? So, for instance, does this trigger a warning? Hopefully not...

class MyClass:
   def __init__(self):
      self.session = aiohttp.ClientSession(...)

async def main():
   obj = MyClass()

asyncio.run(main())

In this case ClientSession is called from a regular function, but there is a main loop running.

[asvetlov]

You are correct. ClientSession initialization is ok when called from an active event loop (asyncio.get_running_loop() from Python 3.7 doesn't raise an exception).

But the session creation outside of asyncio task (module-level code or something executed in low-level asyncio callbacks like Protocol.connection_made()) generates a warning.
The warning will be replaced with strong exception in aiohttp 4.0

[gjcarneiro]

But is there a good reason for requiring a task, instead of just a coroutine, in aiohttp 4.0?

I could create a task to create ClientSession , but there's not guarantee that usage of the ClientSession will always be by the same task that created it.

You seem to be funnelling everything down to the async with ClientSession(...) as session: code style, but I really dislike it, to be honest.

[asvetlov]

I mean ClientSession requires an executed asyncio loop.
Every async function (async def) is executed from a task in asyncio.
asyncio.run() or loop.run_until_complete() create a top-level task imternally.

The session is not pinned to a task. Moreover, running concurrent await session.get() from different tasks is intended behavior. Multiple tasks are ok.

Restrictions are:

1. A session should be created from a running loop
2. The session ownership cannot be translated from one loop to another.
3. The session should be closed before the loop closing.

[gjcarneiro]

Yep, those restrictions make perfect sense to me. Thanks for clarifying!

[krkd]

Hi,
Unfortunately, I believe, it doesn’t solve issue with code organization. I’ve already seen how people still store aiohttp objects in global vars and just creating instances in coroutines.
But it introduces problems into my workflow. I used to separate initialization of app and actual running. As long as we don’t have asynchronous init it is not a problem. Warning is perfectly ok but forcing runtime error pushes me to wrap initialization of app into running Loop or mocking get_running_loop.

[asvetlov]

Maybe now there is a time to change your workflow?
As an option, you can stick with aiohttp 3.x forever.

I don't want to kill global vars, we are all concerned adults.
The change is for preventing aiohttp from hangs if e.g. asyncio.run() is used.

P.S.
I'd like to share an inside.
There is a plan to deprecate asyncio.get_event_loop() in Python 3.9 and drop in eventually. The reason is exactly the same: preventing silly errors when an object is created with one loop but used with another.