Nebo15 / sage

A dependency-free tool to run distributed transactions in Elixir, inspired by Sagas pattern.
MIT License
912 stars 40 forks source link

Return only map of effects after Saga execution is finished #33

Open namxam opened 6 years ago

namxam commented 6 years ago

I am just trying Sage for the first time and therefore not sure if I am using it correctly.

When running a saga, its return value looks like: {:ok, last_effect, effects}. Wouldn't it make more sense to just return {:ok, effects}. As from a consumer's point of view I do not want to know about which step was last and what it's result was.

And where would I format / post process it. Would you put a case statement right after a saga's steps or where the saga is actually called?

AndrewDryga commented 6 years ago

@namxam so last_effect is initially there because for simple sagas you may be only interested in latest return (like with with expression). I think breaking it in when we will reach v1.0 is a good idea.

Yes, you would have a case statement after the execute/2 call. Pretty much the same as you would have it with Ecto.Multi.

chulkilee commented 6 years ago

Should stage names be considered public API? By returning "raw" effects, Sage exposes the stage name to caller.

That would be useful in some cases, but I think it's better to have a way to transform effects to return value, so that that "transformation" holds the public contract.

I may wrap it like this

def run_saga do
  new()
  |> run(:foo, &create_foo/2)
  |> execute(opts)
  |> transform_effects()
end

defp transform_effects()

However, it would be nice if Sage has the built-in way to do this, like:

def run_saga do
  new()
  |> run(:foo, &create_foo/2)
  |> transform_effects(&my_transformer/1)
  |> execute(opts)
end

defp my_transfomer(effects)
AndrewDryga commented 6 years ago

@chulkilee So in your example, it looks like we only add syntax sugar and make Sage more complex and we don't save a single line of client code in a return. The code with explicit call looks more obvious for people that never worked with this library.

The way how I would do it is to pipe Sage execution return to case, like this:

def run_saga do
  new()
  |> run(:foo, &create_foo/2)
  |> execute(opts)
  |> case do
    {:ok, _last_effect, %{user: user}} -> {:ok, user}
    {:error, reason} -> {:error, reason}
  end
end

We used this pattern a lot with Ecto.Multi and Ecto transactions.

chulkilee commented 6 years ago

That looks great! I thought run_saga should ends with Sage.execute/2, but you're right - it can be passed to case and handle there.

BTW, if we switch {:ok, last_effect, effects} into {:ok, effects}, then we loose one info (what was the last one) since Sage does not (and cannot - e.g. for async)store order of effects. However, I agree that it's not needed if the common use of Sage is to pipe into case, where the context is pretty clear

gullitmiranda commented 3 years ago

@chulkilee can't we get the ordered stages from %Sage{} struct?

to create a pattern to return and transmit events from https://github.com/otobus/event_bus (as recommended at https://github.com/Nebo15/sage/issues/23) in order, I use something like:

%{stages: stages} = sage = new()
  |> run(:board, &create_board/2)
  |> run(:task, &create_task/2)

{:ok, _, effects} = sage = new()
  |> execute(opts)

# since stages is a reversed list, i don't need to revert the order at the end
stages 
|> Enum.map(fn name, _ -> name end)
|> Enum.reduce([], fn(name, acc) ->
  case Map.get(effects, name) do
    {_, events} -> [{name, events} | acc]
    _ -> acc
  end
end)