-
Notifications
You must be signed in to change notification settings - Fork 57
Conversation
@philipcmonk I was thinking of building somethign small to try this out. Do you think something like pastebinit would work? |
Yeah, that would be a good project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the seemingly unused path
, not much to say here. This seems in line with previous discussion, and I'm all for this. Style is great, especially useful are the abundance of casts that make it clear what "parts of the monad" we're producing where.
Couple additional nits in here, but no real show-stoppers.
lib/trad.hoon
Outdated
:: notes: notes to send immediately. These will go out even if a | ||
:: later stage of the process fails, so they shouldn't have | ||
:: any semantic effect on the rest of the system. Path is | ||
:: included exclusively for documentation and |verb. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it needs to be updated (maybe even renamed) to remove the remaining vane-ness. Apps don't know about "notes", and the path
doesn't seem like it's used at all (not even included in output).
++ handle-peer ~(handle-peer handler bowl state) | ||
++ handle-diff |=(* (trad-fail:trad-lib %no-diff-handler >path< ~)) | ||
++ handle-take ~(handle-take handler bowl state) | ||
-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repetition of all of the above is a bit meh, but I don't think we have any fancy affordances that could make this better? It's fine as-is, but already shows that it scales pretty poorly. Worth taking note of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have any easy way to say "call this if it exists", and I don't think the core
type is flexible enough to make that work. The other option would be to say that everyone has to implement all the arms in tapp-core-all
. You could say that each arm is either the gate or null, so not handling diff
, take
, and peer would look like this at the bottom of the app:
++ handle-diff ~
++ handle-take ~
++ handle-peer ~
:: | ||
:: Else, fetch the story info | ||
:: | ||
~& "fetching item #{+>:(scow %ui i.top-stories)}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't believe we still don't have a known-to-all, simple way to print "normal" numbers. I'd use ((d-co:co 1) i.top-stories)
here, which feels a little less hacky, but is also more obscure. Hard problems!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
It's worth noting that a lot of the stdio
functions are too restrictive for some of our larger apps, particularly in their handling of HTTP requests/responses. That can be resolved as we apply this pattern more broadly (and furthermore, all of the HTTP-related stdio
functions will need significant revisions when integrated with lighter-than-eyre
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all good to me now. (:
This includes three additional libraries:
lib/tapp
-- A library to create imperative-looking apps via the transaction monad.lib/trad
-- The asynchronous transaction monad.lib/stdio
-- A set of useful asynchronous functions (ie functions that produce values of the asynchronous transaction monad) for various standard requests (HTTP, timers, app pokes and peers, etc)There are also two example apps to demonstrate how these apps can be created. When we have real apps that use these libraries, maybe we should delete these, but for now I think this is the right place for them.
Notice how something like "fetch a web page" happens inline, with no need to cut your app into many functions to handle the different responses. This is the main idea behind these libraries. You can still cut your app by producing a move as an effect rather than in-line (which you want to do if you want to be able to upgrade or process other input before receiving a response), but this makes it easy to not do that when you don't want to.
This also provides a straightforward upgrade approach for situations where you're in the middle of an asynchronous computation. In explicit state machines, you need to write specific code to be able to upgrade at any point regardless of whether you're in the middle of a computation. In practice, we usually ignore this, risking breakage for any user in the middle of a computation. With these imperative-style apps, if you upgrade in the middle of a computation, you can simply cancel the computation and restart it, since computations are transactions that span multiple events.
I'm not super happy with the names here, particularly lib/tapp and lib/trad. For lib/trad, perhaps simply "async" would be better? Then, for example,
form:(async ,@)
would be read as "the type of an asynchronous computation that produces an atom". This name should be short because it's written pretty often. Particularly, any code that uses it will have many functions that start with:A note on style: I use the
=/ m (trad ,return-type)
pattern religiously at the top of each arm that will produce an asynchronous computation. I think this is good because you'll find that you'll callform:m
,pure:m
, andbind:m
often in the middle of the computation, and it's helpful when reading the code because it makes it immediately obvious what the computation produces. This style should be enforced inlib/stdio
and probably any apps that use it.