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

Add SqlTransaction type #692

Merged
merged 5 commits into from
Jun 28, 2022
Merged

Add SqlTransaction type #692

merged 5 commits into from
Jun 28, 2022

Conversation

mschuwalow
Copy link
Member

resolves #665

@mschuwalow mschuwalow requested a review from a team as a code owner May 27, 2022 10:29
import zio.schema.Schema

trait ExprSyntaxModule { self: Jdbc =>
implicit final class ReadSyntax[A](self: Read[A]) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo this end up being the best end-user syntax.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about toZIO ?
imho It seems weird from user perspective to have to call both transact and run.
Ideally if we could omit calling run somehow....how ZIO JDBC does that?

But overall this is great simplification, I'll let also @jczuchnowski review

Copy link
Member Author

@mschuwalow mschuwalow Jun 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure.

The semantics are that you will run the individual statements when you use run on them, but only commit them after the transact scope is done.
I think we should pick naming that makes this very clear.

Maybe withTransaction instead of transact?

@@ -37,4 +44,7 @@ trait Jdbc extends zio.sql.Sql with TransactionModule with JdbcInternalModule wi

def execute(update: Update[_]): ZIO[SqlDriver, Exception, Int] =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to rename this to make clearer that it does not run as part of a transaction

@jczuchnowski jczuchnowski merged commit e4c54c4 into zio:master Jun 28, 2022
@mschuwalow mschuwalow deleted the transactions branch June 30, 2022 10:15
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.

Adopt ZIO JDBC model for transactions
3 participants