LiftIO Instances for Future and Rerunnable
Would LiftIO instances for Future and Rerunnable be welcomed? I couldn't find laws for LiftIO but these rough implementations seemed to work after some limited testing I did.
implicit val rerunnableLiftIO: LiftIO[Rerunnable] = new LiftIO[Rerunnable] {
override def liftIO[A](ioa: IO[A]): Rerunnable[A] =
Rerunnable.fromFuture(LiftIO[Future].liftIO(ioa))
}
implicit val futureLiftIO: LiftIO[Future] = new LiftIO[Future] {
override def liftIO[A](ioa: IO[A]): Future[A] = {
val p = Promise[A]
ioa.unsafeRunAsync {
case Left(ex) => p.setException(ex)
case Right(a) => p.setValue(a)
}
p
}
}
Hi, thanks for the suggestion! If there aren't any laws or other constraints, we should probably piggy back the implementation on the existing functions here:
https://github.com/travisbrown/catbird/blob/master/effect/src/main/scala/io/catbird/util/effect/package.scala
Your implementation will convert the types but stays on the thread pool and we have to clarify what the intended behavior of LiftIO is regarding thread pool shifting.
Concretely: Should it just convert the types from IO while staying on the thread pool of the IO or also shift to the thread pool of the type we converting into (e.g. ContextShift[Rerunnable]).
I'd argue that the latter is what people expect but I couldn't find a definitive answer in the docs. I'll have to double check how cats-effect and monix behave in this regard.
See my recent bugfix which tries to address exactly this problem: https://github.com/travisbrown/catbird/pull/209
In my previous message I might have made some wrong assumptions. One of which is that we already have a function IO[A] => Future[A], but we only have the reverse afaik.
I wrote a message to the cats-effect folks on their opinion but in the meantime I've looked into it more.
It should be noted that Rerunnable already has an instance of LiftIO[Rerunnable] as Effect[Rerunnable] implies Async[Rerunnable] and that implies LiftIO[Rerunnable]. So something like IO.pure("foo").to[Rerunnable] should already work.
For Future the situation is a bit more complicated. We cannot piggyback on Async[Future] as it doesn't exist and one of the laws is that the type may not memoize its result but (to my knowledge) Future does memoize. That's one of the reasons why Rerunnable exists.
Now, we could just implement LiftIO[Future] and ignore Async[Future]. There are pros and cons to it, let's go though some that came to my mind:
Cons:
IO[A]is lazy, butFuture[A]is eager. So the implementation would have to start evaluating the computation, which is a bit more than a simple type-conversion. In your example this is made clear by the use of.unsafeRunAsync. This might not be a dealbreaker, as you'll see further below.- Depending on whether we need to consider any thread pool shifting the
LiftIO[Future]instance might have a depenency on something likeContextShift[Future], which doesn't exist (yet). This might also not be a dealbreaker, just complicates things a bit more.
Pros:
- People who work a great deal inside of applications based on Twitter libraries like Finagle might want to use something that's based on
IO[A], so an instance ofLiftIO[Future]would provide a default conversion implementation that's likely more robust than any ad-hoc implementation they'd otherwise construct. I'm not being condescending here, I'm one of those people (and we have dozens of those conversion functions lying around in the codebase).
I'd like to wait for comments from the cats-effect folks before a final conclusion but currently I think that the practical benefit of having a default conversion implementation (at least for the direction IO[A] => Future[A]) outweighs the cons.
If an application is written in terms of Future[A] there is no expectation that it is referentially transparent, so losing the laziness semantics is not a huge deal in that case.
Running an IO[A] on a future-pool is a bit iffy but I also think this is not a huge problem in practice. I want to do some more research on that but I have a feeling we don't need the context shifting here.
After some discussion on gitter (link) I'd suggest we start with a simple conversion function "unsafeAsyncToFuture" which takes an F[_]: Async and turns it into a Future.
We can document why this function can be unsafe (regarding laziness and thread pool shifting) and what it should be used for. The "unsafe" prefix will hopefully be enough to stop careless usage.
This should be something that works for the tagless-final use-case you described in gitter, @bpholt. Once you verified that it solves your problem, I'd be happy to add it to the project (alongside futureToAsync and the like). What do you think?
Yep, that makes sense to me, except that I think it may need to be Effect instead of Async? This seems to work in place of the LiftIO[Future] I suggested earlier, but I don't see how it can be implemented with only F[_] : Async.
def unsafeEffectToFuture[F[_] : Effect, A](fa: F[A]): Future[A] = {
val p = Promise[A]
Effect[F].runAsync(fa) {
case Left(ex) => IO(p.setException(ex))
case Right(x) => IO(p.setValue(x))
}.unsafeRunSync()
p
}
Yeah, you're right. I wrote Async because that's what we use for the other direction (e.g. futureToAsync).