Add Repo.transaction_with/2

[wojtekmach]

References:

• https://tomkonidas.com/repo-transact/
• https://elixirforum.com/t/seeking-thoughts-on-advantages-of-the-repo-transact-pattern-vs-disadvantages-i-ve-read-about-ecto-multi/61733

I believe the interface and implementation are fairly uncontroversial and straightforward. I think the bigger undertaking is potentially revamping the docs, de-emphasizing Multi in favor of this, given that in a lot of (if not the vast majority of) cases, it achieves the same outcomes with less and simpler code. There are no plans to deprecate Multi.

The name of this function is not ideal, some other options considered were Repo.transact and Repo.with_transaction. I think Repo.transaction_with wins because basically:

iex(1)> Repo.transaction<tab>
transaction/1         transaction/2         transaction_with/1    transaction_with/2

That is, it will show up alongside transaction in docs, autocomplete, etc.

[novaugust]

I'm grateful to see this being brought into Ecto ❤️ it'll help with onboarding new engineers to have transact functionality in ecto's docs rather than our own impls

I'd really like to encourage using transact over transact_with:

• existing codebases, inspired by sasa's post, are already using it (so no need to find-and-replace when updating to latest ecto)
• transact is a verb, same as the existing get/update/delete repo functions (fwiw, we also have the bangified Repo.transact! to fit the theme here)
• _with doesn't really do anything in this name. it feels superfluous in other contexts as well: Enum.map_with for example
• for tab auto completion, i'm going to hit tab well before writing out the whole name,(eg Repo.trans<tab>), which would still give transact alongside all the other existing transaction functions
• transact is typically what developers want over the existing transaction. our codebase has 170 hits for Repo.transact, 66 for transact!, and 2 for transaction - which are just the implementations of transact[!]. giving it a shorter name means they'll see it first and are thus more likely to use it first, whereas the _with suffix means folks are more likely to just see transaction and decide it's the tool they want

Regardless of how it ends up named, thanks for bringing it in!

[sasa1977]

Thanks for bring this to Ecto :heart:

Any thoughts about also supporting returning plain :ok and :error (which would also be the result of transaction_with)? In practice I found this convenient.

[zlatkovlasic]

Very happy to see this, been using Sasa's version for some time. Strongly against proposed function name, tho. Since one of the benefits is simpler and more readable code (with less LOC) - Repo.transaction_with(with ... feels wrong

[Damirados]

I'd really like to encourage using transact over transact_with: ... Regardless of how it ends up named, thanks for bringing it in!

100% to this comment. I will definitely rename it to transact on a project level if it comes in with a different name.

[warmwaffles]

100% to this comment. I will definitely rename it to transact on a project level if it comes in with a different name.

That's fine, but if the function signature is different than the one you expect, what will you do? IE as it stands the anonymous function is required to return {:ok, any()} or {:error, any()} which personally I think is clearer. If you are going to vomit an error, a reason should be provided.

EDIT: Quite a bikeshed moment.

[sasa1977]

FWIW as the original author of the name transact, I'm fine with transaction_with. Given that we're keeping transaction, I actually find that transaction_with better communicates the purpose of the new function.

I think that ideally, the current transaction would become transaction!, and the new function would become transaction, but I guess this isn't possible given the amount of code out there. Alternatively, perhaps transact and transact! with the deprecated transaction, but not sure if it's worth it.

[sodapopcan]

but I guess this isn't possible given the amount of code out there.

Unless Ecto v4? Could ship with a re-writer task, though perhaps I'm oversimplifying this.

[josevalim]

I have discussed this with @wojtekmach:

1. Let's call this transact, it will accept either a function or a multi, the function can return only {:ok, ...} or {:error, ...} for now
2. Let's see if we can implement transaction on top of transact
3. transaction is soft-deprecated (to be fully deprecated later on)

[zlatkovlasic]

Any news on this :)

[wojtekmach]

I hope to finish it in the next week or two, sorry for the delay.

[wojtekmach]

Closing in favor of #4618, thanks everyone for feedback and apologies for delay!