python-trio / triopg

PostgreSQL client for Trio based on asyncpg
Other
45 stars 8 forks source link

How to provide both __await__ and __aenter__ in a single object ? #1

Closed touilleMan closed 6 years ago

touilleMan commented 6 years ago
Help me,Obi-Wan Kenobi. You're my only hope.

(Well if Obi-Wan's not available, @njsmith and @smurfix could be of great help here ^^)

AsyncPG original API contains this create_pool (https://magicstack.github.io/asyncpg/current/api/index.html#asyncpg.pool.create_pool) function that could be use both as an async function and as an async context manager.

However this seems not to work in triopg due to weird interaction with trio-asyncio:

$ py.test triopg/  --runxfail
============================================================================== test session starts ==============================================================================
platform linux -- Python 3.6.1, pytest-3.6.4, py-1.5.4, pluggy-0.6.0
rootdir: /home/emmanuel/projects/triopg, inifile:
plugins: trio-0.4.2, cov-2.5.1
collected 3 items                                                                                                                                                               

triopg/_tests/test_triopg.py .F.                                                                                                                                          [100%]

=================================================================================== FAILURES ====================================================================================
__________________________________________________________________________ test_triopg_pool_with_await __________________________________________________________________________

self = Error(TypeError("trio.run received unrecognized yield message <Future pending>. Are you trying to use a library written for some other framework like asyncio? That won't work without some kind of compatibility shim.",))

    def unwrap(self):
>       raise self.error

venv/lib/python3.6/site-packages/trio/_core/_result.py:119: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
venv/lib/python3.6/site-packages/contextvars/__init__.py:38: in run
    return callable(*args, **kwargs)
venv/lib/python3.6/site-packages/trio/_core/_run.py:936: in init
    self.entry_queue.spawn()
venv/lib/python3.6/site-packages/async_generator/_util.py:42: in __aexit__
    await self._agen.asend(None)
venv/lib/python3.6/site-packages/async_generator/_impl.py:274: in asend
    return await self._do_it(self._it.send, value)
venv/lib/python3.6/site-packages/async_generator/_impl.py:290: in _do_it
    return await ANextIter(self._it, start_fn, *args)
venv/lib/python3.6/site-packages/async_generator/_impl.py:202: in send
    return self._invoke(self._it.send, value)
venv/lib/python3.6/site-packages/async_generator/_impl.py:209: in _invoke
    result = fn(*args)
venv/lib/python3.6/site-packages/trio/_core/_run.py:311: in open_nursery
    await nursery._nested_child_finished(nested_child_exc)
/usr/local/lib/python3.6/contextlib.py:100: in __exit__
    self.gen.throw(type, value, traceback)
venv/lib/python3.6/site-packages/trio/_core/_run.py:196: in open_cancel_scope
    yield scope
venv/lib/python3.6/site-packages/trio/_core/_multierror.py:144: in __exit__
    raise filtered_exc
venv/lib/python3.6/site-packages/trio/_core/_run.py:196: in open_cancel_scope
    yield scope
venv/lib/python3.6/site-packages/trio/_core/_run.py:311: in open_nursery
    await nursery._nested_child_finished(nested_child_exc)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <trio._core._run.Nursery object at 0x7fc9d02f1f28>, nested_child_exc = None

    async def _nested_child_finished(self, nested_child_exc):
        if nested_child_exc is not None:
            self._add_exc(nested_child_exc)
        self._nested_child_running = False
        self._check_nursery_closed()

        if not self._closed:
            # If we get cancelled (or have an exception injected, like
            # KeyboardInterrupt), then save that, but still wait until our
            # children finish.
            def aborted(raise_cancel):
                self._add_exc(Result.capture(raise_cancel).error)
                return Abort.FAILED

            self._parent_waiting_in_aexit = True
            await wait_task_rescheduled(aborted)
        else:
            # Nothing to wait for, so just execute a checkpoint -- but we
            # still need to mix any exception (e.g. from an external
            # cancellation) in with the rest of our exceptions.
            try:
                await checkpoint()
            except BaseException as exc:
                self._add_exc(exc)

        popped = self._parent_task._child_nurseries.pop()
        assert popped is self
        if self._pending_excs:
>           raise MultiError(self._pending_excs)
E           TypeError: trio.run received unrecognized yield message <Future pending>. Are you trying to use a library written for some other framework like asyncio? That won't work without some kind of compatibility shim.

Here we endup with a Future object in the trio event loop. I've guess the way __await__ is implemented (copied on what asyncpg does) doesn't get well with how trio-asyncio does it wrapping

https://github.com/python-trio/triopg/blob/1899510e7aa3665d99fc9d544fff6c8a8cfa915a/triopg/_triopg.py#L103-L111

smurfix commented 6 years ago

Whcih trio-asyncio version are you using? the current master should work better but needs test fixes and a release.

njsmith commented 6 years ago

Before diving into the tricks required to make that work, I want to check– is it necessary? When I've seen this before it's been basically a clever hack to avoid writing async with await something(), and IMO the extra confusion is not worth it, so normal trio style is to not support this.

touilleMan commented 6 years ago

@smurfix From what I see v0.7.5 on pypi and current master are on the same commit (c6c9c542645b5c3502ef3997677abe041f66454d). The bug is present in this version.

@njsmith that makes sense, I'll update the code to drop the broken await support then

touilleMan commented 6 years ago

support dropped by 721255251e9564789d75d4104dc13debaae3cf98

touilleMan commented 6 years ago

On second thought, there is a good reason connect and create_pool should be only available as async context managers: it's the only way for them to make sure they can teardown the asyncio part no matter what. Otherwise we can end up in a strange situation when trio code has been cancelled but asyncio code is still running.

This also means pool.close and connection.close shouldn't be exposed by triopg because they are automatically called when leaving the async context manager.