numberoverzero / bottom

asyncio-based rfc2812-compliant IRC Client
http://bottom-docs.readthedocs.io
MIT License
76 stars 23 forks source link

Client API - should connect/disconnect be coroutines? #23

Closed numberoverzero closed 8 years ago

numberoverzero commented 8 years ago

I've decided to revert the change that made connect/disconnect synchronous but non-blocking, and will again make them coroutines. I expected this to be a tough decision, but after writing out the examples below it seems rather straightforward.

I think the examples are valuable enough for future discussions about the API to keep the issue around, even though I intend to implement the coroutine version immediately.

Waiting for connection to establish after connect()

The coroutine form is the clear winner here, with an explicit "wait until this happens before executing the rest of the body". Contrasting, the other form requires setting an asyncio.Event in a different function and awaiting it in the place we care about it. If any other function can clear/set that event, things become spaghetti quickly.

As a coroutine

@client.on("client_disconnect")
def reconnect(**kwargs):
    await asyncio.sleep(3, loop=client.loop)
    await client.connect()
    client.trigger("reconnect.complete")

Non-blocking non-coroutine

There are a few ways to solve this, but let's use an asyncio.Event for now.

conn_established = asyncio.Event(loop=client.loop)

@client.on("client_connect")
def _mark_connect_complete(**kwargs):
    conn_established.set()

@client.on("client_disconnect")
def reconnect(**kwargs):
    await asyncio.sleep(3, loop=client.loop)
    conn_established.clear()
    client.connect()
    await conn_established.wait()
    client.trigger("reconnect.complete")

Schedule connection without waiting for it to complete

The non-coroutine form wins here, but I don't think it's by much. Because it's so easy to schedule coroutines for an event loop, and Client exposes that loop, it's barely more code for the coroutine version.

As a coroutine

@client.on("client_disconnect")
def reconnect(**kwargs):
    client.loop.create_task(client.connect())
    print("I print immediately")

Non-blocking non-coroutine

@client.on("client_disconnect")
def reconnect(**kwargs):
    client.connect()
    print("I print immediately")