Changed the connect/disconnect coroutines to use a task instead of yield from

[AMorporkian]

Changed the connect/disconnect coroutines to use a task instead of yield from. This is necessary because any blocking coroutines in a triggered 'connect' event will prevent any further triggers from being called.

As a simple example of why this is necessary, the following example should be more than enough to prove the necessity:

@bot.on('CLIENT_CONNECT')
def connect():
    bot.send('NICK', nick=nick)
    bot.send('USER', user=nick, realname='Segmund')
    yield from asyncio.sleep(1000)

[numberoverzero]

Sweet! This looks like a fix for #12 which was going to use accordian, but I never got around to it.

If I'm reading this correctly, won't the event loop need to be passed to asyncio.Task?

Thanks for the PR! I'd be happy to merge once we verify this fixes the issue. I'll see about writing up a test case that currently fails, and passes with the change.

[AMorporkian]

The python docs might warn against using asyncio.Task directly, but asyncio.ensure_future was only added in 3.4.4, which was released less than two weeks ago, so you might not want to use that; you'd be causing errors in a lot of people's bots on upgrade.

[numberoverzero]

I did some work in the loop-refactor branch to get all of the code off of the default event loop, and in revisiting it realized it should be much easier to remove the blocking portions of the code: refactor events.trigger to be non async, and use the ensure_future (or equivalent 3.4.3 call) instead.

This would also fix the issue with Client.run stopping the world to run callbacks, since they would simply enqueue in the event loop.

I'm not convinced a minor bump is a blocker, since the 3.3+ versions are remarkably interchangeable; even 3.5.0 was a perfectly smooth transition. However, I'd be willing to cut the 1.0 version to guard against issues, which should be sufficient for anyone concerned with a minor version bump (surely they're freezing requirements).

[AMorporkian]

Makes sense to me.

By the way, I was thinking about it all day and finally have some tests sort of working. It was trickier than I thought it'd be. I'm going to refine them and have them committed either tonight or tomorrow morning.

[AMorporkian]

Sorry, I didn't make it clear in my comment, I'm happy to change it to ensure_future.

[numberoverzero]

Thanks! I'm just about to open a PR to merge a fix with a unit test, if you want to weigh in.

It's the same fix you proposed here, just backed up to the real problem spot; namely, EventsMixin.trigger.

Every call that was using yield from ...trigger had the same problem, namely waiting on the triggered callbacks to complete. This is a significant change, so I plan to bump us into the 1.0.0 release. Still need to make some documentation changes (trigger is no longer a coroutine)

Edit: #17 is up for review.

[numberoverzero]

Closing since #17 was merged.