Python State Machine version: 2.0.0 5f6bf4ca
Python State Machine version: 1.0.2 ff2e6c53 -ish
- versions installed via pip
Python version: 3.8.10
Operating System: uname -a == Linux belohorizonte 5.15.0-67-generic #74~20.04.1-Ubuntu SMP Wed Feb 22 14:52:34 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Description

365 breaks code that uses callbacks on transitions, when external cycling of states is done via `TransitionList.call()`. (I have tested my system which relies upon 1.0.2, and it does not run.)

The usefulness of cycle = trans1 | trans2 | trans3 | ... to move through state and transition callbacks cannot be overstated. This enables external code to "run" a machine without knowing anything about its states, transitions, or internal representation.

This patch removes a major behavioral feature, enabling external code to cycle a machine instance through states, transitions, and callbacks on both.

From a comment on #308

I added a condition on the callback to only run if the callback was associated with the expected event. So even sharing the same transition instance, the action will be only fired if the event name matches.

If you follow this thought all the way through, wouldn't you need to stop calling callbacks for states, too? Since they also do not share the name of the event, cycle in my case.

I guess the only choice left is to convert my transition callbacks into state callbacks... but, will state callbacks also go away on TransitionList too? Can conditions even be used on states?

What was your motivation for doing this? The "before" behavior seemed correct. The fix seems incorrect, given the prior intent of python-statemachine to allow callbacks at various stages of a machine's lifecycle.

I should be able to get Transition callbacks fired when cycling through a TransitionList. That's why I started using this package, it provided a rich set of callbacks, plus the ability to cycle. I should not have to manually invoke a transition by name in the code that uses my StateMachine subclass. Doing so would violate the principle of Dependency Inversion, which states that external code using an object must depend solely upon the object's abstractions, and not its concrete implementation. Likewise, Separation of Concerns comes to mind-

Separation of concerns results in more degrees of freedom for some aspect of the program's design, deployment, or usage. Common among these is increased freedom for simplification and maintenance of code. When concerns are well-separated, there are more opportunities for module upgrade, reuse, and independent development. Hiding the implementation details of modules behind an interface enables improving or modifying a single concern's section of code without having to know the details of other sections and without having to make corresponding changes to those other sections.

Here is how I currently run one of my machines-

    while not agent.final:
        agent.cycle()

So simple. The code using a machine subclass doesn't need to know anything about its internal workings. It just runs it like a clock. With the current version, that code would need to know the name of every transition, and manually call them one at a time. Am I understanding this correctly?

Here is an outline of that machine subclass (implementation details removed). Its set of states, transitions, and conditions, describe a workflow DAG with conditional branching. I am not sure I could actually get this to work under 2.0.0 just by calling agent.cycle() anymore, because it doesn't seem that State(...)'s constructor provides for conditions, the way that Transition does.

class UpstreamBarsAgent(StateMachine):
    created = State('created', initial=True)
    requested = State('requested')
    received = State('received')
    parsed = State('parsed')
    ingested = State('ingested')
    finished = State('finished', final=True)
    finished_empty = State('finished_empty', final=True)
    request_failed = State('request_failed', final=True)
    receive_failed = State('receive_failed', final=True)
    parse_failed = State('parse_failed', final=True)
    ingest_failed = State('ingest_failed', final=True)

    request = created.to(requested, cond="ready_to_request")
    receive = requested.to(received, cond="ready_to_receive")
    parse = received.to(parsed)
    ingest = parsed.to(ingested, cond="ready_to_ingest")
    finish_empty = parsed.to(finished_empty, cond="is_payload_empty")
    finish = ingested.to(finished)
    request_fail = requested.to(request_failed, cond="did_request_fail")
    receive_fail = received.to(receive_failed, cond="did_receive_fail")
    parse_fail = parsed.to(parse_failed, cond="did_parse_fail")
    ingest_fail = ingested.to(ingest_failed, cond="did_ingest_fail")

    cycle = (request | request_fail |
             receive | receive_fail |
             parse | finish_empty | parse_fail |
             ingest | ingest_fail |
             finish)

    def on_create(self):
        pass

    def ready_to_request(self):
        pass

    def ready_to_receive(self):
        pass

    def ready_to_ingest(self):
        pass

    def did_request_fail(self):
        pass

    def did_receive_fail(self):
        pass

    def did_parse_fail(self):
        pass

    def did_ingest_fail(self):
        pass

    def is_payload_empty(self):
        pass

    def on_request(self):
        pass

    def on_receive(self):
        pass

    def on_parse(self):
        pass

    def on_ingest(self):
        pass

    def on_finish(self):
        pass

    def on_finished_empty(self):
        pass

    def on_request_fail(self):
        pass

    def on_receive_fail(self):
        pass

    def on_parse_fail(self):
        pass

    def on_ingest_fail(self):
        pass

Hi @Kevin-Prichard , how are you? Sorry to hear that this change impacted your use case. Let's try to fix things up.

I started to think that firing callbacks of distinct events was an issue when working on this example: https://python-statemachine.readthedocs.io/en/2.0.0/auto_examples/traffic_light_machine.html

It just doesn't feel right to call a callback directly associated with an event name when firing another event.

So the fact that request = created.to(requested, cond="ready_to_request") returns a TransitionList can be interpreted as an implementation detail. The semantics of this line means that you're binding an event request to all transitions on the right side of the assignment.

So this:

class UpstreamBarsAgent(StateMachine):
    created = State(initial=True)
    requested = State()
    received = State()
    parsed = State()

    request = created.to(requested, cond="ready_to_request")
    receive = requested.to(received, cond="ready_to_receive")
    parse = received.to(parsed)

    cycle = (request | receive | parse)

    def on_request(self):
        pass

    def on_receive(self):
        pass

    def on_parse(self):
        pass

Has the same semantics as this:

class UpstreamBarsAgent(StateMachine):
    created = State(initial=True)
    requested = State()
    received = State()
    parsed = State()

    created.to(requested, event="request cycle" cond="ready_to_request")
    requested.to(received, event="receive cycle", cond="ready_to_receive")
    received.to(parsed, event="parse cycle")

    def on_request(self):
        pass

    def on_receive(self):
        pass

    def on_parse(self):
        pass

This puts in perspective what I mean: You can have multiple events associated with a transition. When they're bounded by name, it seems only the actions associated with the name should be called.

The solution to your use case is to bind the event directly to the transition, instead of to the event.

So without reverting the "fix", we have two options to bind actions directly to transitions instead of events.

Bind using decorator syntax

class UpstreamBarsAgent(StateMachine):
    created = State(initial=True)
    requested = State()
    received = State()
    parsed = State()
    ingested = State()
    finished = State(final=True)
    finished_empty = State(final=True)
    request_failed = State(final=True)
    receive_failed = State(final=True)
    parse_failed = State(final=True)
    ingest_failed = State(final=True)

    request = created.to(requested, cond="ready_to_request")
    receive = requested.to(received, cond="ready_to_receive")
    parse = received.to(parsed)
    ingest = parsed.to(ingested, cond="ready_to_ingest")
    finish_empty = parsed.to(finished_empty, cond="is_payload_empty")
    finish = ingested.to(finished)
    request_fail = requested.to(request_failed, cond="did_request_fail")
    receive_fail = received.to(receive_failed, cond="did_receive_fail")
    parse_fail = parsed.to(parse_failed, cond="did_parse_fail")
    ingest_fail = ingested.to(ingest_failed, cond="did_ingest_fail")

    cycle = (request | request_fail |
             receive | receive_fail |
             parse | finish_empty | parse_fail |
             ingest | ingest_fail |
             finish)

    def on_create(self):
        pass

    def ready_to_request(self):
        pass

    def ready_to_receive(self):
        pass

    def ready_to_ingest(self):
        pass

    def did_request_fail(self):
        pass

    def did_receive_fail(self):
        pass

    def did_parse_fail(self):
        pass

    def did_ingest_fail(self):
        pass

    def is_payload_empty(self):
        pass

    @request.on
    def do_request(self):
        pass

    @receive.on
    def do_receive(self):
        pass

    @parse.on
    def do_parse(self):
        pass

    @ingest.on
    def do_ingest(self):
        pass

    @finish.on
    def do_finish(self):
        pass

    @finish_empty.on
    def do_finished_empty(self):
        pass

    @request_fail.on
    def do_request_fail(self):
        pass

    @receive_fail.on
    def do_receive_fail(self):
        pass

    @parse_fail.on
    def do_parse_fail(self):
        pass

    @ingest_fail.on
    def do_ingest_fail(self):
        pass

Bind by params:

class UpstreamBarsAgent(StateMachine):
    created = State(initial=True)
    requested = State()
    received = State()
    parsed = State()
    ingested = State()
    finished = State(final=True)
    finished_empty = State(final=True)
    request_failed = State(final=True)
    receive_failed = State(final=True)
    parse_failed = State(final=True)
    ingest_failed = State(final=True)

    request = created.to(requested, cond="ready_to_request", on="do_request")
    receive = requested.to(received, cond="ready_to_receive", on="do_receive")
    parse = received.to(parsed, on="do_parse")
    ingest = parsed.to(ingested, cond="ready_to_ingest", on="do_ingest")
    finish_empty = parsed.to(finished_empty, cond="is_payload_empty", on="do_finish_empty")
    finish = ingested.to(finished, on="do_finish")
    request_fail = requested.to(request_failed, cond="did_request_fail", on="do_request_fail")
    receive_fail = received.to(receive_failed, cond="did_receive_fail", on="do_receive_fail")
    parse_fail = parsed.to(parse_failed, cond="did_parse_fail", on="do_parse_fail")
    ingest_fail = ingested.to(ingest_failed, cond="did_ingest_fail", on="do_ingest_fail")

    cycle = (request | request_fail |
             receive | receive_fail |
             parse | finish_empty | parse_fail |
             ingest | ingest_fail |
             finish)

    def on_create(self):
        pass

    def ready_to_request(self):
        pass

    def ready_to_receive(self):
        pass

    def ready_to_ingest(self):
        pass

    def did_request_fail(self):
        pass

    def did_receive_fail(self):
        pass

    def did_parse_fail(self):
        pass

    def did_ingest_fail(self):
        pass

    def is_payload_empty(self):
        pass

    def do_request(self):
        pass

    def do_receive(self):
        pass

    def do_parse(self):
        pass

    def do_ingest(self):
        pass

    def do_finish(self):
        pass

    def do_finished_empty(self):
        pass

    def do_request_fail(self):
        pass

    def do_receive_fail(self):
        pass

    def do_parse_fail(self):
        pass

    def do_ingest_fail(self):
        pass

Another way to see this last syntax alternative is that you don't even need to declare the other events if they are not meant to be called directly... so they are not real "events" (something that happens from the outside world).

This also works:

class UpstreamBarsAgent(StateMachine):
    created = State(initial=True)
    requested = State()
    received = State()
    parsed = State()
    ingested = State()
    finished = State(final=True)
    finished_empty = State(final=True)
    request_failed = State(final=True)
    receive_failed = State(final=True)
    parse_failed = State(final=True)
    ingest_failed = State(final=True)

    cycle = (
        created.to(requested, cond="ready_to_request", on="do_request")
        | requested.to(received, cond="ready_to_receive", on="do_receive")
        | received.to(parsed, on="do_parse")
        | parsed.to(ingested, cond="ready_to_ingest", on="do_ingest")
        | parsed.to(finished_empty, cond="is_payload_empty", on="do_finish_empty")
        | ingested.to(finished, on="do_finish")
        | requested.to(request_failed, cond="did_request_fail", on="do_request_fail")
        | received.to(receive_failed, cond="did_receive_fail", on="do_receive_fail")
        | parsed.to(parse_failed, cond="did_parse_fail", on="do_parse_fail")
        | ingested.to(ingest_failed, cond="did_ingest_fail", on="do_ingest_fail")
    )

    def on_create(self):
        pass

    def ready_to_request(self):
        pass

    def ready_to_receive(self):
        pass

    def ready_to_ingest(self):
        pass

    def did_request_fail(self):
        pass

    def did_receive_fail(self):
        pass

    def did_parse_fail(self):
        pass

    def did_ingest_fail(self):
        pass

    def is_payload_empty(self):
        pass

    def do_request(self):
        pass

    def do_receive(self):
        pass

    def do_parse(self):
        pass

    def do_ingest(self):
        pass

    def do_finish(self):
        pass

    def do_finished_empty(self):
        pass

    def do_request_fail(self):
        pass

    def do_receive_fail(self):
        pass

    def do_parse_fail(self):
        pass

    def do_ingest_fail(self):
        pass

So, if this works for you, I think that we win the possibility of having the two outcomes. Then I update the release notes adding this as a breaking change with these options as upgrade paths.

Best regards!

fgmacedo / python-statemachine

#365: No longer invoking transition callbacks breaks things #368

Description

365 breaks code that uses callbacks on transitions, when external cycling of states is done via `TransitionList.call()`. (I have tested my system which relies upon 1.0.2, and it does not run.)

Bind using decorator syntax

Bind by params:

fgmacedo / python-statemachine

#365: No longer invoking transition callbacks breaks things #368

Description

365 breaks code that uses callbacks on transitions, when external cycling of states is done via TransitionList.__call__(). (I have tested my system which relies upon 1.0.2, and it does not run.)

Bind using decorator syntax

Bind by params:

365 breaks code that uses callbacks on transitions, when external cycling of states is done via `TransitionList.call()`. (I have tested my system which relies upon 1.0.2, and it does not run.)