Skip to content

Add Repo.prepare_transaction/2 #4605

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

Conversation

milmazz
Copy link
Contributor

@milmazz milmazz commented Apr 14, 2025

What I'm doing

Adding a new callback prepare_transaction/2 to Ecto.Repo.

Why?

To offer the possibility of further modifying the given Ecto.Multi or options in transaction operations before it is transformed and sent to the database. @josevalim suggested to consider adding this function to inject the commit_comment option if the users wanted to extract that comment based on the stacktrace.

Approach

  1. Create the new prepare_transaction/2 callback.
  2. Provide a default implementation of the callback.
  3. Invoke that callback from the transaction operation in Ecto.Repo.
  4. Add unit tests.
  5. Update the Ecto.TestAdapter to include the opts given to the transaction operation.
  6. Update broken unit tests (after the previous change).

Alternate approaches considered

As mentioned in this discussion, if the user wants to inject a commit_comment based on the stacktrace, the only option available today is the following hack:

defoverridable transaction: 1, transaction: 2

def transaction(fun_or_multi, opts \\ []) do
  # The opts[:stacktrace] option is not available at this point, so, we probably have to invoke
  # Ecto.Repo.Supervisor.tuplet/2 twice. Sad times :-(
  # Is that, or do we avoid the `super` at the end and do everything ourselves here?
  opts = Keyword.put_new_lazy(opts, :commit_comment, fn -> extract_comment(opts) end)

  super(fun_or_multi, opts)
end

And we certainly don't want to promote these things ;)

I’d like feedback on

The current implementation of prepare_transaction/2 takes a fun_or_multi as the first argument, but it doesn't make too much sense to accept the fun part (a function with arity zero or one) because I don't see how that would be useful or how it can be extended. The original implementation of prepare_transaction/2 accepts a fun to keep things as simple as possible. As always, I'm open to suggestions.

@josevalim josevalim merged commit b5e6877 into elixir-ecto:master Apr 18, 2025
13 of 14 checks passed
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

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

Successfully merging this pull request may close these issues.

2 participants