Trio concurency backend PoC #214
Conversation
|
This is awesome, thanks for being the first to take a stab at this! :) Something I think we ought to do for a trio backend: have some way we can run all our test cases with the trio backend instead of the asyncio backend so we can double the value of our test suite easily. :) This'll definitely need @tomchristie's eyes as well. |
Absolutely. I copy-pasted the async client tests knowing that it was definitely not ideal. I think making use of pytest fixtures and using the backend inside fixtures/tests instead of asyncio/trio would allow something like this. |
|
Ah fabulous, great stuff!
Indeed. I'm open to suggestions here around how best to marshal support for alternate backends, but this PR is a great start! |
|
So, having looked a bit more into this, I think a necessary step before pulling off a complete trio backend would be to have end-to-end usage of the concurrency backend in the whole code base. This requires to remove remaining asyncio-specific code, in particular in |
|
This now contains the changes from #217, and there's progress: only tests for the blocking client are now failing! 🎉 (Again because of a Edit: talked too soon — still having issues with timeouts. I suspect there's a bug in my trio read implementation. Need more investigation… |
|
@sethmlarson Today's update: I realized that the timeouts I was having were due to incorrectly handling the necessary coexistance of the asyncio and trio event loops. Since the uvicorn server has to run on asyncio (because uvicorn is asyncio-only), I figured that:
So what I did is add a pytest hook that automatically defers to the asyncio threadpool tests that use the The hook works like a charm on async tests, but I'm having issues with the blocking ones, e.g. Any clues on this? Am I missing a crucial piece of info that would allow simplifying things considerably? |
|
Your guess is as good as mine. Gonna cc the I/O wizard: @njsmith |
|
I don't really understand what you've been trying – it sounds complicated! – and I can't debug a TrioInternalError without any details :-). So uh, here's some general comments?
|
|
Thanks for pitching in @njsmith! Here's a minimal repro example for the # app.py
import asyncio
import httpx
from httpx.contrib.trio import TrioBackend
async def app(scope, receive, send):
assert scope["type"] == "http"
await send(
{
"type": "http.response.start",
"status": 200,
"headers": [[b"content-type", b"text/plain"]],
}
)
await send({"type": "http.response.body", "body": b"Hello, world!"})
def test_get(backend):
url = "http://127.0.0.1:8000/"
with httpx.Client(backend=backend) as http:
response = http.get(url)
assert response.status_code == 200
async def main():
loop = asyncio.get_event_loop()
await loop.run_in_executor(None, test_get, TrioBackend())
if __name__ == "__main__":
asyncio.run(main())Run it with:
Full traceback below: The issue seems to be with running I am seemingly getting the same error if slimming down the repro example to just this: import asyncio
import trio
def test_get():
async def gen():
yield "hi"
g = gen()
trio.run(g.__anext__)
async def main():
loop = asyncio.get_event_loop()
await loop.run_in_executor(None, test_get)
if __name__ == "__main__":
asyncio.run(main())Traceback: Is it an issue to run a step of an async generator with |
|
Filed and debugged at: python-trio/trio#1191 (The problem is that Trio is not expecting you to have an async function that's implemented in C, because those are extremely rare, and it's crashing while trying to do some introspection that doesn't work on C code. Then because of some annoying details the error gets lost.) We can fix that but there's a more fundamental problem: the only way it makes sense to call I assume that this is happening because httpx's sync API is using a strategy where it allocates an asyncio loop and then calls |
|
Hmm, now that you say it, what would prevent us from always using asyncio for the sync API? I don’t see a use case where we’d use the sync client instead of the async one in a trio (async) context anyway. I believe the backend argument on Client was added in #60 — any thoughts on this @tomchristie? Also, thanks for debugging this @njsmith! |
|
I think you're totally right on the sync client not accepting a backend @njsmith. Thanks for all the debugging! :) |
Refs #120
For now I basically copy-pasted the asyncio backend from
concurrencymodule and replaced asyncio-specific parts with the trio equivalent. Did the same thing for the async client tests to get a quick idea of how things were going.Tests are read timing out for now, I'll try and look as to why that is the case tomorrow. :-)