reactioncommerce / hooks

The event hooks API for Reaction Commerce
MIT License
2 stars 1 forks source link

Add a version that will do the equivalent of `Promise.all` #1

Closed brent-hoover closed 5 years ago

brent-hoover commented 5 years ago

We need to be able to run all event handlers and then run a function afterwards.

So possibly a version that is Hooks.Events.addAsync and then Hooks.Events.runAsync that runs all the handlers and then can run a function.

Use Case: After I import a product, I want other modules to be able to run transformations on that imported product, but after they are all done, I want to publish the product.

We could just use Promise.all if we ensured that all functions added to Hook.Events returned a promise. Or we could use something like async.parallel from the async library.

aldeed commented 5 years ago

@zenweasel So, a few related points:

  1. FYI, I am planning to (probably this week) remove all use of Hooks and replace with appEvents. This is one of the last breaking things we want to get merged prior to 2.0 final release.
  2. For hook-like functions that we need to await, we have been trying to register functionsByType and explicitly call and await those in core plugins. Can you add a new function type for this, which other plugins can be aware of and register?
  3. In the interest of future-proofing, we should think hard about whether synchronous hooks like this can be avoided. Because they'll work less and less well, as services are split and kafka is introduced. In some cases, such as customer just placed the order and different services need to check and calculate different things, we can't avoid needing to be synchronous. But in the case of import, it seems to me that this could probably be done mostly with async events. Something like this:
    • Importer service/plugin creates products and emits "product import done"
    • Transformations run based on listening for "product import done", and emit "transformation 1" is done. (If these can't be run in parallel or order matters, then you might need a single transformation service that in turn calls functions registered with functionsByType in a certain order.)
    • Publisher service/plugin listens for "all transformation done" or is somehow aware of which transforms we expect to run and whether they've all run. Then publishes.

Point 3 is just an idealistic goal. If there isn't time to figure out how to do it all with events, then I'd go for functionsByType solution.

brent-hoover commented 5 years ago

@aldeed re point #3. That is what we were looking for but how does any particular event know it's the last one and doesn't that break encapsulation? I would also prefer an event-driven flow however it's t's the "somehow aware of which transforms we expect to run and whether they've all run" that made me fallback to using synchronous code.

Whether you are awaiting or responding to an event it seems like it's more of a semantic difference than an actual difference in how the tasks would be scheduled. Also, in this case we want the item to be published even if any of these transformations fail so it still seems like the caller needs to be responsible.