python-statemachine icon indicating copy to clipboard operation
python-statemachine copied to clipboard

Transition - validate against model/machine

Open arseniy-panfilov opened this issue 6 years ago • 8 comments

  • Python State Machine version: 0.7.1
  • Python version: 3.6.
  • Operating System: macOS 10.14.6

Description

Hi!

I'm trying to figure out how to include the machine's model or even the machine itself into validation parameters - let me illustrate it by an example.

Assume we have this machine with a single "run" transition and a class representing a runner. Of course running requires some energy, so a side effect of it is, naturally, a decrease in energy:

from statemachine import StateMachine, State

class RunningMachine(StateMachine):
    start = State('start', initial=True)
    end = State('end')

    run = start.to(end)

    def on_run(self):
        self.model.energy -= 10

class Runner:
    def __init__(self, energy):
        self.energy = energy
        self.state = 'start'

runner = Runner(15)
RunningMachine(runner).run()
print(runner.energy)  # 5
print(runner.state)  # end

So far so good! Now what I'd like to do is to make sure that Runner has enough energy to run in the first place, so I add a validation like this:

def has_enough_energy(runner):
    assert runner.energy >= 10

class RunningMachine:
    ...
    run = start.to(end)
    run.validators = [has_enough_energy]

The problem is that in order to use it I now have to pass a Runner instance as an argument for transition, otherwise it won't make it to the validation function:

class RunningMachine:
    ...
    def on_run(self, runner):
        runner.energy -= 10

machine = RunningMachine(runner)
machine.run(runner)  # wait a second, don't you have reference to the 'runner' already?

It can get even worse since the runner I pass into it might in fact be a different runner! Which of course leads to an inconsistency:

fresh_runner = Runner(10)
tired_runner = Runner(0)

machine = RunningMachine(tired_runner)
machine.run(fresh_runner)  # oops...

print(fresh_runner.energy)  # 0
print(fresh_runner.state)  # start
print(tired_runner.state)  # end 

What I Did

Well there's not much I can do about it but make sure to pass the same Runner instance to the transition itself. It doesn't come handy and raises some issues I've outlined above so it all comes down to the following questions:

  • Am I missing something / doing something wrong? I.e. is there a (better) way to include the model/machine into the validation process?
  • If there's none, validators functionality could probably be improved by allowing str arguments, e.g.:
class RunningMachine:
    run = a.to(b)
    
    def can_run(self):
        assert self.runner.energy >= 10
    run.validators = ['can_run']    

or by allowing class method (this would require some checks so that transition would know if it should pass model to the validator in order to not break compatibility):

class RunningMachine:
    def can_run(self):
        assert self.model.energy >= 10
    
    @start.to(end, validators=can_run)
    def run(self):
        self.model.energy -= 10

Of course I'd be happy to create a PR implementing either of these approaches - or any other if that would make more sense to you.

Thanks!

arseniy-panfilov avatar Sep 12 '19 15:09 arseniy-panfilov

Hello @arseniy-panfilov,

Maybe it's a bit late to come with a solution. Anyway, I don't know if I have understood correctly, but you want to check if a certain condition is applied after performing a transition.

Why dont you just override the constructor?

class RunningMachine(StateMachine):
    start = State('start', initial=True)
    end = State('end')

    run = start.to(end) 

    def __init__(self, energy, **kwargs):
        super(RunningMachine, self).init(**kwargs)
        self.energy = energy

    [...]

claverru avatar Oct 22 '19 08:10 claverru

Hi @arseniy-panfilov, thanks for pointing this issue. Currently, validators have the same api as the transition callback, except for not accepting the statemachine as the first param. They should raise an exception in order to interrupt flow.

I'm inclined to deprecate validators in favor of a new design based on predicates, methods that return a boolean indicating if the transition can run or not. Something like guards.

fgmacedo avatar Jan 23 '20 14:01 fgmacedo

Hi @claverru, thanks for your response and sorry for a late reply! Could you please elaborate on the constructor idea? In the example above I could probably do

def __init__(self, runner):
    self.energy = runner.energy

but would it actually change anything? I mean I already have access to runner.energy via machine's self.model property, which is the runner itself - so I can write self.model.energy. But it doesn't help with validators since they do not have access to the state machine nor its model. I could make validation part of transition handler like this:

def on_run(self):
    if self.model.energy < 10:
        raise ConditionNotMet("not enough energy")

but it's not very reusable - that's why I'd like to move it over to validators

arseniy-panfilov avatar Jan 23 '20 16:01 arseniy-panfilov

Hi @fgmacedo, thanks for your reply! Indeed I ended up having predicates, and then I wrapped them into a callable which would raise an error in case any of these predicates returns False; but I still have to pass StateMachine's model to transitions, so it doesn't solve the original issue 😞

For the reference, my solution with predicates looks like this now:

# define predicates
def has_enough_energy(runner) -> Bool: pass
def has_enough_motivation(runner) -> Bool: pass

# predicates wrapper which would throw an exception - compatible to StateMachine's validator api
class Condition:
    def __init__(self, predicates: List[Callable[[Runner], Bool]]):
        self.predicates = predicates

    def __call__(self, runner):
        if not any(predicate(runner) for predicate in self.predicates):
            raise StateMachineError

# use in StateMachine like this
run.validators = [Condition(has_enough_energy, has_enough_motivation)]

def on_run(self, runner):
    runner.energy -= 10

arseniy-panfilov avatar Jan 23 '20 16:01 arseniy-panfilov

Hello @arseniy-panfilov,

I needed a while to retake this issue, and I still don't know if I understood the problem nor if my solution is good.

I think that what I was trying to propose involved converting the State Machine to the runner itself. In the snippet

from statemachine import StateMachine, State

class RunningMachine(StateMachine):
    start = State('start', initial=True)
    end = State('end')

    run = start.to(end)

    def on_run(self):
        self.model.energy -= 10

class Runner:
    def __init__(self, energy):
        self.energy = energy
        self.state = 'start'

runner = Runner(15)
RunningMachine(runner).run()
print(runner.energy)  # 5
print(runner.state)  # end

you have a runner which is handled by a state machine. I proposed converting the state machine to the runner, by overriding the constructor so that he can have energy as argument (keeping the native StateMachine constructor's aguments), and avoiding the creation of the class Runner.

Let me know if this is valid for you. If not, explain to me like I'm five to see if I can understand xD.

Regards!

claverru avatar Jan 23 '20 16:01 claverru

Hi @arseniy-panfilov , nice snippet! It really shows that something is missing. We can't keep up with this validators API.

Let me share what I'm thinking about.

I'm reading about statecharts since SCXML (Statechart XML), is a W3C standard and it defines a lot of the semantics and specifies how to deal with certain edge cases. A statechart is essentially a state machine that allows any state to include more machines, in a hierarchical fashion. This is already a feature request at https://github.com/fgmacedo/python-statemachine/issues/246.

With that in mind, my goal to this library may turn to be as much as possible compatible with statecharts but using a different syntax. A pythonic interface to statecharts.

So, related with your issue, we should implement 'guard'.

fgmacedo avatar Jan 23 '20 18:01 fgmacedo

Hi @claverru! Thanks for your continued attempts to solve my issue! :) apparently I'm not that good when it comes to highlighting the important stuff hehe; anyway, let me elaborate one more time.

My main and only issue is adding validators to transitions. Even if I convert my StateMachine into Runner (which I'd like to avoid - I prefer my models being simple), it doesn't solve validation issue since validators as they are right now do not have access to model nor state machine itself. Instead, they only have access to the same arguments you pass into transition.

E.g. In this example

def on_run(self, foo):
    pass

if you add a validator to the on_run transition, your validator will only receive foo argument and that's it. Therefore, if you want to validate against StateMachine's model, you have to pass this model as an argument to the transition, which might potentially break data consistency (see my previous examples).

arseniy-panfilov avatar Feb 18 '20 02:02 arseniy-panfilov

May I join the conversation? Validation is a generic word. I talked before with @fgmacedo and the library networkx appeared. Graphs are very useful and transitions must be possible.

The use case you mentioned might be possible, but validate with some self wrapper seems an adequate manner to solve it. Also possible, although verbose, is to provide some validation callback which prints or pop out some notification. Other reasonable feature is to provide possible transitions as a metadata. Validation edge to edge is also possible, although algorithms like Dijkstra would require external library. Self-implementation is hard. :'(

brunolnetto avatar Feb 10 '21 22:02 brunolnetto

I was walking around, so I decided to verify these issues. Have you come to a conclusion? Maybe we may close this issue one for all with a trivial validation example.

I learned from these post reading-through that:

  1. We will somewhen implement predicative on gardé with runtime error raise on transition execution. The conditional statement leads to try-catch requirement from the caller side and hence error handling, e.g., logging;
  2. The provided example by @arseniy-panfilov on post gives some implementation hints. Are they sufficient? I am afraid, they are not.

Great! I suppose the roadmap is:

  1. Review the current state of the art on validators;
  2. Find the main code modifications for the desired feature;
  3. Remove legacy code by substitution of the desired feature;
  4. Provide tests for common and corner cases;
  5. Update the version in Semantic versioning syntax. Suggested: major;
  6. Notify users from previous versions about breaking changes.

brunolnetto avatar Dec 10 '22 20:12 brunolnetto

Hi! After a while, I devised a working idea for implementing this without breaking everything else. But It's a huge refac.

Please let me know what you think and if it works for your use case: https://github.com/fgmacedo/python-statemachine/pull/294

cc @arseniy-panfilov

fgmacedo avatar Dec 28 '22 22:12 fgmacedo