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

Machines get stuck and don't stop #40

Closed
greenrd opened this issue Mar 15, 2015 · 9 comments
Closed

Machines get stuck and don't stop #40

greenrd opened this issue Mar 15, 2015 · 9 comments

Comments

@greenrd
Copy link
Contributor

greenrd commented Mar 15, 2015

This is an informal pull request, because I've created my commits against v0.4.1 in a new 0.4.1 branch which doesn't exist in your repo, so I can't create a proper pull request.

Please see https://github.com/greenrd/machines/tree/0.4.1

I have two commits there, one fixing a bug relating to repeatedly, and the other relating to fanout - both bugs cause machines to not stop when they should. Well, I'm sure the fanout one is a bug, anyway; I haven't actually verified that the repeatedly change fixes a bug, but it makes sense to me.

@ekmett
Copy link
Owner

ekmett commented Mar 15, 2015

The version of repeatedly that appears to have been live when you wrote those patches looks like it was just horribly horribly broken.

It should have had the same semantics as construct . forever all along.

@ekmett
Copy link
Owner

ekmett commented Mar 15, 2015

So I think the repeatedly changes are effectively made already.

The other is the change to fanout. For that I'm going to either defer to @acowley about the semantics of that operation (as he wrote it) or take it upon myself when I have more time and a clearer head.

@greenrd
Copy link
Contributor Author

greenrd commented Mar 15, 2015

It seems I misunderstood the Haddock comment for repeatedly then. It says:

Generates a model that runs a machine until it stops, then start it up again.

But it doesn't start it up again:

Prelude Data.Machine Control.Applicative Control.Monad> take 10 <$> runT (repeatedly $ yield "hello" >> stop) :: IO [String]
["hello"]

@ekmett
Copy link
Owner

ekmett commented Mar 15, 2015

Ah. I see the description difference.

The word "stop" there came from before we added the Stop constructor!

repeatedly (yield "hello")

should run as intended. It really is intended to run until the plan returns at the end.

@greenrd
Copy link
Contributor Author

greenrd commented Mar 15, 2015

OK, thanks, I understand now. I pushed some more commits. I avoided rewriting the git history because that might have been confusing.

@acowley
Copy link
Contributor

acowley commented Mar 15, 2015

The fanout change probably makes sense; I've not focused enough on machines that stop. It would be good to write some tests (doctests maybe) to demonstrate stopping behavior as it is a bit murky in most places. Or perhaps we need to do a documentation audit to standardize terminology so we can concisely say what happens when we, say, fanout before upstream stops, but downstream processes don't all stop at the same time.

@ekmett
Copy link
Owner

ekmett commented Mar 15, 2015

@acowley: Makes sense. Volunteering? ;)

@bens
Copy link
Contributor

bens commented Dec 9, 2015

I just saw this ticket and thought I'd mention that #70 should fix the fanout part of it.

@ekmett
Copy link
Owner

ekmett commented Dec 9, 2015

With both repeatedly and fanout doing what they should I'm going to close this out. Please re-open if it is still an issue.

@ekmett ekmett closed this as completed Dec 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants