terricain / aioboto3

Wrapper to use boto3 resources with the aiobotocore async backend
Apache License 2.0
719 stars 74 forks source link

Feature request: Non-async shutdown #274

Open couling opened 1 year ago

couling commented 1 year ago

Problem description

The use of an async context manager as the only way into having a client can be very awkward to work with, especially where the client is likely to be long lived for performance reasons (see also #236).

Besides from wanting to hold connections open for a long time, there's a common convention of having close() methods not coroutines. There's an example of this model in core python where async streams have a def close() method and an accompanying async def wait_closed(): https://docs.python.org/3/library/asyncio-stream.html#asyncio.StreamWriter.close

So often enough I find myself having to write a def close() method that is forced to ultimately invoke aioboto3 client's __aexit__(). The only workaround I am left with is to write something like:

class Foo:
    def close(self):
        # Schedule it and hope!  
        asyncio.get_running_loop().create_task(self._async_close())

    async def _async_close(self):
        ...

Questions?

Feature request

In an ideal world it would be very helpful if:

Since the current way in is only through the async context manager there's no reason why this suggestion could not remain backwards compatible. I'm not suggesting removing the existing behaviour of __aenter__ and __aexit__.

terricain commented 1 year ago

So, this was done long enough ago that I can't remember exactly why, but it was along the lines of there were some pretty extensive changes to how aiobotocore was initialised and this was the easiest way to make it work and ensure aiobotocore's contexts were closed too.

I fully agree, its a pain, even using asyncexitstack's dont help much. If you can get it working without breaking backwards compatibility and it not looking too nasty, by all means, raise a PR.

josebsneto commented 1 year ago

Has this problem been resolved?

dacevedo12 commented 1 year ago

somewhat related https://pypi.org/project/asyncio-atexit/

couling commented 1 year ago

I'm starting to have second thoughts about how useful this might be.

More recent python versions (3.11) remove the loop parameter from a lot of async objects and the removal of implicit loop creation all point to a different way of working.

Prior to python 3.10/3.11, many people wrote a lot of the setup code to execute before the event loop started. But with no implicit loop creation and no way to pass in a loop parameter, much of the setup code for core python classes must happen in an async loop.

This is leading to a common pattern to have two methods def main() and async def async_main() with one used to setup an event loop and the other to use it for async startup and shutdown.

In good time I believe non-async shutdown will become an anti-pattern.

bk5115545 commented 5 months ago

While I would never actually recommend it, per https://peps.python.org/pep-0492/#await-expression, __await__() is compatible with the generator protocol.

So from a sync context, the following is valid:

list(client.__aexit__().__await__())

Note: this does block and probably has some major problems that I'm missing but it I've used it in AWS Lambda to ensure successful shutdown of long-lived clients for years so far without issue.