Allow arbitrary parameters to be passed to event callbacks

[matt-jordan]

Often, it's useful to allow arbitrary data to be passed to event callbacks. For example, take handling a playback finished event initiated from a channel playback:

def on_playback_finished(playback, ev):
    pass

playback = channel.play(media='sound:tt-monkeys')
playback.on_event('PlaybackFinished', on_playback_finished)

What if we wanted to take a further action on the channel? Unfortunately, Python has some quirks with closures. If all we want to do is read from the channel, we can reference the channel directly and Python will understand that the variable is not local:

def on_playback_finished(playback, ev):
    channel.play(media='sound:tt-weasels')

However, if what we had instead was a counter - such as something keeping state on what media sound file to create - Python will assume that the variable we are writing to is local to the function, and will behave badly:

sounds = ['tt-monkeys', 'tt-weasels']
current_sound = 0

def on_playback_finished(playback, ev):
    # NOPE! This is now a local variable. Kerblooey.
    current_sound += 1
    play_next_sound()

def play_next_sound(channel):
    playback = channel.play(media='sound:%s' % sounds[current_sound])
    playback.on_event('PlaybackFinished', on_playback_finished)

While this example is contrived, it is common for an ARI client user to want to maintain state, and to pass that state along to callbacks. While lambda functions can sometimes be used, it often creates a "two-pass" system of defining an anonymous callback function that does nothing but call the real handler. Generally, it's often much easier to just call the callback function directly and pass it the arguments you want it to take as args/*kwargs to the event registration function.

This patch allows the following to be done (taken from an in-process example):

sounds = ['press-1', 'or', 'press-2']

class MenuState(object):
    def __init__(self, current_sound, complete):
        self.current_sound = current_sound
        self.complete = complete

def play_intro_menu(channel):
     menu_state = MenuState(0, False)

    def play_next_sound(menu_state):
        if (menu_state.current_sound == len(sounds) or menu_state.complete):
            return None
        try:
            current_playback = channel.play(media='sound:%s' % sounds[menu_state.current_sound])
        except:
            current_playback = None
        return current_playback

    def on_playback_finished(playback, ev, menu_state):
        queue_up_sound(channel, menu_state)

    def queue_up_sound(channel, menu_state): 
        current_playback = play_next_sound(menu_state)
        if not current_playback:
            return

        menu_state.current_sound += 1
        current_playback.on_event('PlaybackFinished', on_playback_finished,
                                  callback_args=[menu_state])

The primary purpose for this is to chain playbacks as they occur, such that a user pressing a DTMF key can cancel the playback immediately. This requires keeping the state of the menu in the callbacks, the easiest method being to pass that state into the callbacks directly. Note that the callback_args is taken by ari-py and stored with the event registration. When the event is received and the callback fired, the callback_args is expanded as *args and passed to the callback handler.

Because we have to store the arguments with the function to be called, ari-py now uses a list instead of a set to store the information (as a tuple is not hashable). In order to not change the behaviour, registration of a duplicate function causes the original handler to be removed.

[putnopvut]

I like the new functionality, but I don't like the inconsistent use of argument packing. In your example, you have

current_playback.on_event('PlaybackFinished', my_callback, callback_args=[foo, bar], callback_keywords = {'baz': True})

but if I were writing an ari-py program where I call on_channel_event on the client, I'd call it as

client.on_channel_event('StasisStart', my_callback, foo, bar, baz=True)

Any particular reason for the inconsistency? If I could have it my way, I'd have *args and **kwargs everywhere.

[samuelg]

What's you opinion as far as setting the version to 0.1.3 as part of this pull request? Should we keep that as a separate commit?

[matt-jordan]

Punting on this, as Sam's pull request is better :-)