numberoverzero / bottom

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

Example await connect #33

Closed numberoverzero closed 7 years ago

numberoverzero commented 7 years ago

From additional feedback in #28, this is almost entirely a README + docs refactor. There is a code change in examples/echo.py, so I bumped the patch version to 4.

Thanks again to @thebigmunch for detailed feedback.

meshy commented 7 years ago

(Comment moved from https://github.com/numberoverzero/bottom/issues/28.)

There is an interesting parallel between d5425da and https://github.com/gawel/irc3/issues/14.

That issue on irc3 lists waiting for MOTD as a bug because it causes errors when connecting to bouncers. Perhaps it's worth adding a third wait to cover that circumstance? I don't personally use a bouncer, so I'm not exactly sure what to recommend, but PRIVMSG springs to mind. Perhaps there is something more appropriate. I hope that's helpful :)

numberoverzero commented 7 years ago

Hm. This could be covered by passing the timeout parameter to asyncio.wait. This is becoming more handling than I want to expose in the sample, since it's not bottom-specific. I think I'd split it out of the README and into bottom.utils or such.

The new sample becomes:

...

from bottom.util import wait_for_first

# Any of the following would indicate we can now join channels
join_after = [
    "RPL_ENDOFMOTD",
    "ERR_NOMOTD",
    "PRIVMSG",
    "PING"
]

@bot.on("client_connect")
async def connect(**kwargs):
    ...
    await wait_for_first(bot, *join_after, timeout=5)
    bot.send("JOIN", ...)
import asyncio

async def wait_for_first(client, *signals, timeout=None):
    """Waits for one of any number of signals to occur.
    Returns the str of the first signal that occurred, or
    None if a timeout was provided and it has been exceeded.
    """
    # No signals to wait on, immediately return
    if not signals:
        return

    done, pending = await asyncio.wait(
        futures=[client.wait(signal) for signal in signals],
        loop=client.loop,
        timeout=timeout
    )

    for unfinished in pending:
        unfinished.cancel()

    if done:
        # TODO how to get signal name here?
        return done.SOMETHING
meshy commented 7 years ago

That looks like a nice solution to me :)

thebigmunch commented 7 years ago

Don't feel like working around an SSL verify issue due to self-signed cert for my ZNC instance tonight, or I'd do some testing there.

As another user posted in the linked irc3 issue that it's working for them, it's likely that the OP has the block_motd module enabled and the later commenter does not. ZNC does send a message to the client with the text "MOTD blocked by ZNC" when that module is enabled, though.

That being said, I'm fine with the ability to wait on multiple events.

numberoverzero commented 7 years ago

Cool. I think this will become a minor bump, since Client.wait will need to (somehow) return the string of the signal that was waited for.

Or some other mechanism to plumb it back. Time to read up on asyncio.Event...

thebigmunch commented 7 years ago

So I lied, curiosity forced me into checking it out. Even with the block_motd module enabled, ZNC sends a 422 'ERR_NOMOTD' message to the client that seems to be caught fine by bottom.

Edit: The irc3 issue was opened up ~2 years ago. Perhaps this was changed/fixed in ZNC since then.

numberoverzero commented 7 years ago

Fair enough. I'm going to keep the sample as-is, and add another python example that includes the helper. Turns out, it's way easier to associate events than I thought; they're hashable, which means they are perfectly fine dict keys.