From 323df635d97cd249e52cf426b060959a9f77bc8f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 1 Dec 2016 14:30:38 -0400 Subject: forward comment and add comment --- ...ent_2_60d6e06ebada37648df77442733e325f._comment | 24 +++++++++++ ...ent_3_45413e6e811c34edc38a6ff70ca7c208._comment | 49 ++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_2_60d6e06ebada37648df77442733e325f._comment create mode 100644 doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_3_45413e6e811c34edc38a6ff70ca7c208._comment (limited to 'doc') diff --git a/doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_2_60d6e06ebada37648df77442733e325f._comment b/doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_2_60d6e06ebada37648df77442733e325f._comment new file mode 100644 index 00000000..3233340c --- /dev/null +++ b/doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_2_60d6e06ebada37648df77442733e325f._comment @@ -0,0 +1,24 @@ +[[!comment format=mdwn + username="chris" + subject="""comment 2""" + date="2016-12-01T18:14:10Z" + content=""" +Agree on all points. I would rather not add the dependencies to propellor +proper either, but such was the requirement for this change. I'd be happy +enough with the MonadBase IO derivation and implementing this externally, +no argument here. + +As for what it does :) I cribbed the implementation from the Snap server ( +https://github.com/snapframework/snap/blob/ +bda15d0a0f29b0107fd69fbb8b7e8cc5ce5fa7e4/src/Snap/Snaplet/Internal/Types.hs# +L277), +and it seems to work, essentially it is a way to take the outer +transformer, and wrap it inside the inner Monad, but in such a way that the +inner Monad now has access to the outer transformer !? Yeah, I'm still not +fully grokking it myself, but it type checks and functions. + +Anyway feel free to implement at your leisure, it does seem that I could +even derive the MonadBase IO instance manually and not have to change +Propellor in the least, though the auto-derived instance would seem like a +simple and harmless addition. +"""]] diff --git a/doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_3_45413e6e811c34edc38a6ff70ca7c208._comment b/doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_3_45413e6e811c34edc38a6ff70ca7c208._comment new file mode 100644 index 00000000..58106f64 --- /dev/null +++ b/doc/todo/Add_MonadBaseControl_instance_to_Propellor/comment_3_45413e6e811c34edc38a6ff70ca7c208._comment @@ -0,0 +1,49 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 3""" + date="2016-12-01T18:14:28Z" + content=""" +Looking at the lifted-async that is what uses the MonadBaseControl instance +in your use case, I have some concerns. + +Its docs say "All the functions restore the monadic effects in the forked +computation unless specified otherwise." I think that has bearing on the +following situation: + +Suppose that two Propellor monad actions are run concurrently by this: + + foo `concurrently` bar + +Propellor's monad includes a Writer component, that accumulates [EndAction]. +Since they are running concurrently, it seems likely that `foo` and `bar` +are using separate Writers. Propellor doesn't currently use a State monad, +but suppose that was added to its stack. Then `foo` and `bar` would +necessarily, I think, be manipulating independent copies of state. + +Now, what happens when `concurrently` finishes running them? We have two +Writers and/or two States, that need to be merged somehow. I don't see +anything in the library that lets it do an intelligent merge. (For example, +it could notice that [EndAction] is a monoid and mappend the two values.) + +So, I think when it says it's arestoring the monadic effects, it means it's +*discarding* any changes that might have been made to the Writer or State. + +Is this a large problem for propellor? Maybe not. EndActions rarely need to +be added, and in fact only one property in all of Propellor currently adds +an EndAction. + +Now, I actually dealt with this problem in the +Propellor.Property.Concurrent module. The code there threads the Writer +values through the concurrent actions and merges them at the end. If +MonadBaseControl provides a more principled way to do that, which lets +lifted-async also be used safely, then that part of propellor could perhaps +be changed to use it. + +But, I don't know if this is a problem that MonadBaseControl deals with at +all. It might be that its design is intended to be used for things like +`bracket`, where there's no concurrency, and so not as much problem with +getting different monadic states that need to be merged together. (Although +in `bracket foo bar baz`, if baz throws an exception part way through, +there's an interesting question about what to do with any monadic state it +may have accumulated.) +"""]] -- cgit v1.2.3