-
Notifications
You must be signed in to change notification settings - Fork 346
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
Zio #1989
Zio #1989
Conversation
|
||
def transaction[A](f: Task[A]): Task[A] = { | ||
val dbEffects = for { | ||
connectionRef <- currentConnection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about ZIO.environment[Connection] instead of FiberRef?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamgfraser suggested that too. Is we’re going in that direction, should run
return a task with a Connection environment as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, if we’re gonna do that, should we just rely on the user doing their own bracketing of the connection?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And also, should we take environment of PreparedStatement in the cases that we use it. Or if we’re managing it ourselves should we bracket it?
|
||
override protected def withConnection[T](f: Connection => Task[T]): Task[T] = | ||
for { | ||
maybeConnectionRef <- currentConnection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it be wrapped in effectBlocking
?
// TODO Are we really catching the result of the conn.rollback() Task's exception? | ||
catchAll(Task(conn.rollback()) *> Task.halt(cause)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of catching rollback errors, maybe one possible alternative could be to combine the causes (like with ++
for example) since Cause
can represent multiple failures.
I'd also be willing to help with this, but I don't want to get in the way. Maybe like upgrading to ZIO 1.0.3 later and fixing the breaking changes (though you might not run into too many). There's nitpicky stuff like "you can use shorter combinators like |
This is so exciting! |
@reibitto Any help is highly appreciated, if you'd like to work on this with me or take over please let me know. Also, I'm not sure about blocking. Right now the effect type is |
3caa15b
to
4bda10e
Compare
The Whether it makes sense to create a For reference, there's tranzactio which uses Besides the questions above, what are the remaining parts? I see tests commented out, but is there anything else? If the main ZIO modules are pretty much done I can try giving it a spin in my app and seeing what happens. If that works fine, I can maybe try exploring the other ideas I mentioned above. |
4b73e66
to
735907d
Compare
|
||
// Primary method used to actually run Quill context commands query, insert, update, delete and others | ||
override protected def withConnectionWrapped[T](f: Connection => T): RIO[BlockingConnection, T] = | ||
blocking(RIO.fromFunction((conn: BlockingConnection) => f(conn.get))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
f(conn.get)
cloud throw exception. Would be better wrapper that with some effect
case Success(_) => | ||
UIO(conn.get.commit()) | ||
case Failure(cause) => | ||
UIO(conn.get.rollback()).foldCauseM( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UIO.apply
expect an unexceptional effect. Those jdbc calls will throw SQLException
Okay, this has been a while in the making but we seem to be done. Quill ZIO effects how have the following signature:
Unlike many other contexts, the ZIO Quill contexts do not require connections or data-sources to be passed inside. Since they return a ZIO that encapsulates the dependency, they can be completely static:
I have provided various helper functions in order in order to make this process easier, these are in
ZioJdbc.scala
For an example, a simple ZIO app using JDBC goes as follows:
The
zioConn
part can be significantly simplified by usingLayers
inZioJdbc.scala
.One important thing to note is that for the streaming methods (i.e.
context.stream
which returns aZStream
), an option to chunk the stream itself is provided. This is based on a method calledchunkedFetch
which is based onZStream
'sfromIterator
with the significant difference that a single effect is not a single row but a chunk instead. The actual chunking logic is done byguardedChunkFill
. Hopefully future releases ofZStream
will incorporate this functionality so it will not be necessary to maintain here.Also, it is worthwhile to note that the
context.prepare
functions also now have a zio idiomatic signature:These methods are allow a user to get down into the JDBC layers upon executing a query if this is needed.