Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mention in doc auto_transitions=False must be set to be able to use the Error State extention #318

Closed
potens1 opened this issue Oct 24, 2018 · 3 comments

Comments

@potens1
Copy link
Contributor

potens1 commented Oct 24, 2018

Since the implementation relies on machine.get_triggers, the machine must be created with auto_transitions=False because the test that check the state is final will never be True because, if all the to_ methods exists, the state is never final.

I don't know if this should be mentioned in the doc or if the implementation must be changed to filter all the on_ (seems not very reliable to me either), or add a tag when accepted: False on the state, or a Final tag (meaning, it's final and not accepted)

@aleneum
Copy link
Member

aleneum commented Oct 24, 2018

The auto_transitions keyword is just a convenient way to create a bunch of to_<state> transitions. They behave like every other transition apart from that. Since there is no restriction considering how automatically generated transitions can be used, they should imho be treated as ordinary transitions.

However, I do agree this limits the usefulness of state.Error for cases in which auto transitions are intended to act as 'dev/admin' tools. We have implemented a filter function for diagrams here which might be helpful to filter out transitions that 'look like' auto transitions. What about a keyword ignore_auto_transitions for state.Error? It increases the effort required for Error initialisation but tackles the issue locally. What do you think?

@potens1
Copy link
Contributor Author

potens1 commented Oct 24, 2018

I'm not very fond of the filtering idea, the problem I see is if you create to_ method by your self, it can bites (i.e, you are not very imaginative, and every trigger you create in the transition, you call it to_). My point of view on that is it should be enough to put a big fat warning in the doc saying Error is exclusive with auto_transitions. I'm fan of the Principle of least astonishment

@aleneum
Copy link
Member

aleneum commented Oct 24, 2018

My point of view on that is it should be enough to put a big fat warning in the doc saying Error is exclusive with auto_transitions

fair enough. I added a note to state.Timeout and state.Error which hopefully expressed both extensions' limitations. Thanks again for your feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants