when_file_changed handlers may be lost

[stub42]

This may be the general case of Issue #25

Several handlers decorated by when_file_changed may be scheduled for an iteration. If any handler removes state, handlers that should no longer be triggered are removed. Unfortunately, this includes all the pending when_file_changed handlers, as the file checksums now match and the check is now False.

In this example, the bounce_foo handler will not run if the do_something handler is invoked first.

@hook('config-changed')
def changed():
    write('/etc/foo.conf', config)
    set_state('need_something')

@when('need_something')
def do_something():
    print('Do something')
    remove_state('need_something')

@when_file_changed('/etc/foo.conf')
def bounce_foo():
    print('Bounce foo')

Resetting the hashes would be tricky, so maybe the best approach is to have the when_file_changed decorator set some internal state so that the check always returns true until the handler as been run and the state reverted.

[johnsca]

More and more I think that re-testing the queue when a state is removed is a mistake.

It was put in there to try to guard against the case of a handler taking some action that negated the state but then a subsequent handler taking action as if the state were still set and that causing an error. However, there's just as much of an issue if two handlers have @when_not conditions on a state and one of them adds the state in question.

Additionally, it doesn't seem like the framework can both guard against that case and ensure that all handlers get their chance to run when their conditions are met, and the latter seems like the more important goal.

Finally, if that sort of co-dependence exists between handlers, it seems likely to be a design flaw and even so the handler could always manually check the state again with is_state if absolutely necessary.

[galgalesh]

This seems like a tricky case. I think retesting the queue is the correct thing to do. Consider the following use-case:

@hook('config-changed')
def changed():
    write('/etc/foo.conf', config)
    set_state('need_something')

@when('need_something')
def do_something():
    print('Do something')
    remove_state('service.running')

@when_file_changed('/etc/foo.conf')
@when('service.running')
def bounce_foo():
    restart_service()

Restarting a stopped service is not something you want to do. However, the current way when_file_changed is handled doesn't seem correct to me. Maybe this might be a solution:

Consider when_file_changed an event. A handler that has an event decorator can only be put on the queue at the time the event fires. If an event fires and the handler's conditions are met, the handler is put on the queue. While cleaning the queue, only states are checked, not events. I think this behavior covers both mine and stub42's use-case.?