Fix some code #2928
Fix some code #2928
Conversation
| protected val identityExtractor = (rr: ResultRow, s: Session) => rr | ||
| protected val identityPrepare: Prepare = (p: PrepareRow, _: Session) => (Nil, p) | ||
| private val _identityExtractor: Extractor[Any] = (rr: ResultRow, _: Session) => rr | ||
| protected def identityExtractor[T]: Extractor[T] = _identityExtractor.asInstanceOf[Extractor[T]] |
There was a problem hiding this comment.
Improves type inference of the identityExtractor usage
guizmaii
force-pushed
the
fix/zio-2.0.17
branch
from
3560eec
to
65c1d85
Compare
| private[getquill] val streamBlocker: ZStream[Any, Nothing, Any] = | ||
| ZStream.scoped { | ||
| for { | ||
| executor <- ZIO.executor | ||
| blockingExecutor <- ZIO.blockingExecutor | ||
| _ <- ZIO.acquireRelease(ZIO.shift(blockingExecutor))(_ => ZIO.shift(executor)) | ||
| } yield () | ||
| } |
There was a problem hiding this comment.
This function/code was probably incorrect and was probably not doing anything.
The ZIO.acquireRelease(ZIO.shift(blockingExecutor))(_ => ZIO.shift(executor)) code/execution takes action in the returned Scope.
Here, we were opening and closing a new Scope for the execution of this code. This means that, after this code, the Scope would already have been closed, would already have executed its finalizer and so would already have executed the "shift back to the previous executor" code.
The code following this function call would be then executed on the "previous executor", not on the blocking one.
I decided to remove this function and replace it with ZStream.blocking because I don't see a good reason for us to change the behaviour/semantics of ZIO's "blocking" mechanism.
ZIO might adopt a "shift back immediately after" or a "continue on the same thread" behaviour for some code wrapped in a [ZIO|ZStream].blocking(...) (FYI, in ZIO 2, it's "continue on the same thread")
The user might decide to yield back or not. I don't see why we're taking this decision to always yield back
@deusaquilus Any specific reason? 🤔
Staying with the default mechanism provides more control to the user who might want to yield back or stay on the same thread. I don't think it's up to us to decide.
guizmaii
force-pushed
the
fix/zio-2.0.17
branch
from
65c1d85
to
1ad7bf6
Compare
| case Some(size) => | ||
| ZStream.fromIterator(iter, size) | ||
| case None => | ||
| ZStream.fromIterator(new ResultSetIterator(rs, conn, extractor)) |
There was a problem hiding this comment.
The ResultSetIterator was instantiated twice when fetchSize was None
guizmaii
force-pushed
the
fix/zio-2.0.17
branch
from
d5e8781
to
1616506
Compare
guizmaii
force-pushed
the
update/zio-2.0.17
branch
from
3b34975
to
ab27a27
Compare
guizmaii
force-pushed
the
fix/zio-2.0.17
branch
from
1616506
to
c34a07f
Compare
guizmaii
changed the title
|
|
guizmaii
changed the title
|
Needs zio/zio#8501 |
guizmaii
force-pushed
the
fix/zio-2.0.17
branch
from
a64e498
to
8c59f83
Compare
guizmaii
force-pushed
the
fix/zio-2.0.17
branch
from
d9054ef
to
5dbf43a
Compare
|
@guizmaii can I merge it or would you like to wait for Alex review? |
|
@juliano If you're okay with it, I think we can merge it. Let's give @deusaquilus a week to tell us if he wants to review it first, maybe? |
* Update zio, zio-streams to 2.0.17 * Fix tests * Fix tests * simplify code * Fix tests (#2930) * Fix tests * Fix tests * clean * fix tests * clean * Remove `streamBlocker` function * clean * Update ZIO to v2.0.19 * Clean * clean * clean --------- Co-authored-by: zio-scala-steward[bot] <145262613+zio-scala-steward[bot]@users.noreply.github.com> Co-authored-by: Juliano Alves <von.juliano@gmail.com>
This PR initially started to understand why update to ZIO 2.0.17 was making a test to fail.
In the end, the issue was in ZIO and has been fixed in ZIO 2.0.19.
During the bug research phase, I found some issues in the code, which are the changes in this PR.