Implementation for cats-effect 3 #188
Conversation
| @@ -79,17 +79,17 @@ sealed abstract class Gauge[F[_]]{ | |||
| object Gauge { | |||
|
|
|||
| // Convenience | |||
| def incIn[E, F[_]: Bracket[?[_], E], A](g: Gauge[F], fa: F[A]): F[A] = | |||
| Bracket[F, E].bracket(g.inc)(_ => fa)(_ => g.dec) | |||
| def incIn[E, F[_]: MonadCancel[*[_], E], A](g: Gauge[F], fa: F[A]): F[A] = | |||
There was a problem hiding this comment.
I think it's even a pattern in CE3 itself to use MonadCancel[*[_], _] in these situations (if E isn't part of the signature otherwise).
Also I think *[_] isn't supported by -Ykind-projector in dotty so might just want to pass an explicit implicit :)
There was a problem hiding this comment.
Hope I understood explicit implicit thing correctly here :)
There was a problem hiding this comment.
Ah, i meant passing the MonadCancel instamce using an implicit clause, as opposed to a context bound. Sorry for the confusion.
That way it'd be MonadCancel[F, _] I think.
There was a problem hiding this comment.
Yes, this is how I read this and moved MonadCancel to implicit parameter with the second type parameter unlocked.
| @@ -203,7 +203,7 @@ object Gauge { | |||
| case x: UnlabelledGaugeImpl[_, _] => x.underlying | |||
| case x: MapKUnlabelledGauge[_, _, _] => asJavaUnlabelled(x.base) | |||
| } | |||
| def asJava[F[_]: ApplicativeError[?[_], Throwable]](c: Gauge[F]): F[JGauge] = c match { | |||
| def asJava[F[_]: ApplicativeError[*[_], Throwable]](c: Gauge[F]): F[JGauge] = c match { | |||
| Bracket[F, E].bracket(Clock[F].monotonic(unit)) | ||
| {_: Long => fa} | ||
| {start: Long => Clock[F].monotonic(unit).flatMap(now => h.observe((now - start).toDouble))} | ||
| def timed[E, F[_]: MonadCancel[*[_], E]: Clock, A](h: Histogram[F], fa: F[A], unit: TimeUnit): F[A] = |
There was a problem hiding this comment.
Try fa.timed, it's in CE syntax: typelevel/cats-effect#1677
| val test = for { | ||
| cr <- CollectorRegistry.build[IO] | ||
| gauge <- Gauge.noLabels[IO](cr, Name("boo"), "Boo Gauge") | ||
| defer <- Deferred[IO, Unit] | ||
| fib <- gauge.incByIn(defer.get, 10).start | ||
| _ <- Timer[IO].sleep(1.second) | ||
| _ <- Temporal[IO].sleep(1.second) |
|
|
@kubukoz There is a difference between the old implementation of Timed using Example app: import cats.effect.{ExitCode, IO, IOApp}
import io.chrisdavenport.epimetheus.{CollectorRegistry, Histogram, Name}
import scala.concurrent.duration._
object SummaryApp extends IOApp {
override def run(args: List[String]): IO[ExitCode] =
for {
r <- CollectorRegistry.build[IO]
hist <- Histogram.noLabels(r, Name("hist"), "help")
_ <- Histogram.timedSeconds(hist, IO.sleep(4.3.seconds)
.map(_ => throw new RuntimeException()))
.handleErrorWith(_ => IO.unit)
_ <- r.write004.map(println)
} yield ExitCode.Success
}This will have no data in registry for I can see reasons to have measured successes only and all the effects, so, for me it is not clear which one is more valid than another. May be @ChristopherDavenport could comment and then I will update the scaladoc part. Or may be we should expose both APIs for the user and may be even promote bracket version to ce3. 🤷 |
|
Good catch, I didn't think of that. It's probably best to keep the timing method as it was, then. |
|
Reverted |
|
Any blockers on this one? |
|
Looks good to me – thank you @arixmkii 👍 Hope it gets merged soon-ish, I came here because epimetheus (and -http4s) is one of my last blockers on CE3 adoption. |
|
Can we get a milestone with this? :) |
Fixes #182
Important version bumps:
Additionally fixed some warnings generated by
source3checks.