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

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

Open
namxam opened this issue Jun 28, 2018 · 5 comments
Open

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

namxam opened this issue Jun 28, 2018 · 5 comments
Milestone

Comments

@namxam
Copy link

namxam commented Jun 28, 2018

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 AndrewDryga added this to the v1.0 milestone Jun 29, 2018
@AndrewDryga
Copy link
Member

@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.

@AndrewDryga AndrewDryga changed the title Question: Saga response Return only map of effects after Saga execution is finished Jul 7, 2018
@chulkilee
Copy link

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
Copy link
Member

AndrewDryga commented Aug 7, 2018

@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
Copy link

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
Copy link

@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 #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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants