Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.Sign up
the client connect(nursery) API is odd #20
I don't like this API either, but I'm new to Trio and not really sure what else to do.
If the client starts its own nursery, then AFAIK the caller needs to provide a handler coroutine that we will launch as a new task in our own nursery. In this hypothetical, the client code would need to look like this:
Is this something we like? Is it "Trionic" (the Trio version of Pythonic)? cc: @njsmith
referenced this issue
Sep 14, 2018
John pointed out that Fuyukai/asyncwebsockets doesn't take a nursery as an argument. Indeed, it doesn't use a nursery at all. It doesn't need a background task because it doesn't read network data until you call
I also think that this will break if two different tasks call
I agree that trio-websocket has a higher level API and we want it to handle ping and hide wsproto details. So the issue is that if the user is only blocking on next_event() sometimes, then we'd miss a ping.
Regarding client usage by simultaneous tasks, I don't think it should be supported. It's straightforward to wrap a single-task websocket client to support multiplexing, and I've done exactly that with asyncwebsockets. I'm not sure what the semantics of allowing multiple listeners would be. In my case responses need to get paired with requests based on a proprietary scheme, so custom multiplexing is needed regardless.
Regarding passing in nurseries, I've got over a month of using Trio full time now and am forming an opinion that it's an anti-pattern. The usual pattern when you have an object which needs to run something in the background is to give full control to the user about what scope that is run in, most importantly of which is the ability to wrap the task in a try-catch. This error handling part, along with scoped cancellation, are Trio's main selling points, and passing in a nursery foils them.
async def do_session(): try: async with open_websocket(host, port) as ws: # At this point we know that there was successful ws handshake with peer. # We can check ws.port if it was automatically assigned. # ping requests will be handled automatically. # We can call ws.send() and ws.receive(). Neither support # simultaneous task access, so use a lock for send() and multiplexing # on receive() as necessary. # If we're defining an asynccontextmanager, we would # yield ws here. # this point is never reached since open_websocket always raises except ClosedConnection: # pass or something based on close reason except SomeHandlerError: # ... # do_session would normally be run in parallel with other tasks via nursery. # # Another use case is to wrap the run in "with trio.move_on_after()", e.g. if # session can't complete in 30 seconds, give up.
My suggestion would be to have a "low level" API where you pass in a nursery, plus a "high level" API like
The high level API makes things convenient, since most users just want to open a websocket connection and don't care about the details; the low level API makes more exotic cases possible, like when you want to have an unpredictable number of connections with unpredictable lifetimes, and there's no clear mapping between connections and tasks. There's some more discussion of this (even using websocket as the example :-)) here: https://stackoverflow.com/questions/48282841/in-trio-how-can-i-have-a-background-task-that-lives-as-long-as-my-object-does