Bike shedding about cats-effect HOT 15 CLOSED

IO is missing the equivalent of Task#async, which moves a computation to an execution context. I'd also like to get start somehow (without pulling in Ref).

If we have IO#async, the equivalent of Task.apply would become IO { ... }.async which seems really nice and clear.

from cats-effect.

• Remove cats.effect.Attempt alias. It's currently private[effect] which is confusing for folks reading the code. We could make it public but I don't think an effect library should provide a minor syntax convenience like this.

Agreed.

• Replacing Catchable with MonadError means we lose this method:

https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/ApplicativeError.scala#L46

• We need unsafeRunAsync somewhere

Why? fa.unsafeRunAsync(cb) is roughly equivalent to fa.runAsync(cb).unsafeRunSync(), modulo the fact that cb returns Unit in the former and IO[Unit] in the latter. In fact, it's relatively trivial to implement the former in terms of the latter.

• Not sure how I feel about StdIO -- seems like a bucket of random things for which there's no guiding principle on deciding what should be added. E.g., how about randomUUID: IO[UUID]?

I too am unsure of how I feel about it. Happy to remove. The guiding principle was basically "things from System and Runtime that I personally deem useful". Not a great principle.

• Change IO from a sealed trait to a sealed abstract class to help with binary compat issues in the future.

👍

• Note that we are switching to scala.util.control.NonFatal instead of fs2.util.NonFatal. I'm fine with this but both Paul and Daniel supported using a custom notion of NonFatal that didn't catch ControlException and some others.

Oh, I forgot about that! I'll look into the differences.

• Probably should think of a new name for IO#ensuring given the ensuring from Predef

Eh… There's no namespace conflict, since it's being called on an IO instance, and I can't think of anyone other than Bill Venners who uses ensuring. I actually forgot that it even existed until recently.

Do you have a better name in mind though? I'm not married to ensuring, it was just there.

• To make sure I've got it right, Task.delay is now IO.apply and Task.apply doesn't exist, right?

Yep.

IO is missing the equivalent of Task#async, which moves a computation to an execution context. I'd also like to get start somehow (without pulling in Ref).

I wanted to steer clear of any concurrency support in IO itself. Both the functions you're referencing can be implemented in terms of what's here. The forked async constructor is by far the most tempting for me, since most of the time users want to move back onto the main thread pool after a callback. I would be pretty leery of start though.

Maybe the "strict concurrency avoidance" plan is a bad one, I'm not sure. I wanted to do that both to avoid stepping on fs2/Monix, and because I thought the separation of concerns could be beneficial.

from cats-effect.

No strong opinions on ensuring -- I forgot about it until recently too.

I can live without start on IO but I would like a way to shift a synchronous computation to an execution context. How about this?

final def async(implicit ec: ExecutionContext): IO[A] = IO.async { cb => 
  ec.execute(new Runnable { def run = unsafeRunAsync(cb) })
}

from cats-effect.

I'm ok with that, though the name seems a bit weird. How does fork or shift strike you?

from cats-effect.

Agreed on name being weird. I'm fine with either of your suggestions.

Are you okay with both IO.fork and IO#fork existing? The constructor variant would match the implementation you have in the ScalaDoc for async. Or perhaps IO.fork constructor should exist like you have in the docs while IO#shift would match the example above. Either way, I find both of these operations common enough that I'd like them to exist with first class syntax -- i.e., I don't want to have to teach folks how to derive these each time this question comes up.

from cats-effect.

I have an IO.fork constructor in the docs?

from cats-effect.

https://github.com/djspiewak/cats-effect/blob/c5024ed3b2345d481a51c0b947f920cbfaf65b41/shared/src/main/scala/cats/effect/IO.scala#L330-L337

from cats-effect.

Oh right, lol. The constructor variant is basically Task.apply. Is that really something you do so regularly that IO(action).shift is worse than IO.fork(action)? I actually almost never use Task.apply.

from cats-effect.

Eh that's fair. OK I'm happy with just IO(action).shift then. :)

from cats-effect.

Alrighty. :-) So we're loosening the "no concurrency" restriction to "no concurrency, but thread pool manipulation isn't really concurrency, so it's ok," which actually seems pretty reasonable to me.

from cats-effect.

So to summarize, we've got the following actions:

• Add IO#shift
• Remove StdIO
• classify IO
• Investigate NonFatal
• Remove Attempt

Thoughts on my unsafeRunAsync response?

from cats-effect.

What would awakeEvery look like?

https://github.com/functional-streams-for-scala/fs2/blob/20208de46e9ab968726f2a9699b781a6f8da0c49/core/shared/src/main/scala/fs2/time/time.scala#L47

I guess it would just be:

F.runAsync(signal.set(d))(_ => IO(running.set(false))).unsafeRunSync

from cats-effect.

Yep. Conceptually here, the typeclass is "imbuing" IO with special properties. It's effectively saying that IO is the only parametrically valid root of an effect stack, in that any side-effect managing type constructor should be able to asynchronously interpret to it. The function serves the dual purpose of allowing a safe unsafeRunSync on JavaScript, since the resulting IO is guaranteed to have only synchronous actions.

from cats-effect.

@mpilquist Implementing shift, I realized that it is probably best if we implement both versions of it:

  /**
   * Shifts the computation of any prefix contiguous synchronous actions into the implicitly
   * specified `ExecutionContext`.  Asynchronous actions and continuations will be unshifted
   * and will remain on whatever thread they are associated with.  This should be used if
   * you want to evaluate a given `IO` action on a specific thread pool when it is eventually
   * run.
   */
  final def shiftPrefix(implicit EC: ExecutionContext): IO[A] = {
    IO async { cb =>
      EC.execute(new Runnable {
        def run() = self.unsafeRunAsync(cb)
      })
    }
  }

  /**
   * Shifts the continuation of the action into the implicitly specified `ExecutionContext`.
   * The thread pool association will be observed in any actions which are flatMapped onto
   * the result of this function.  That is to say, the *continuation* of the `IO`.  All of
   * the effects within the current `IO` will retain their pre-existing thread affinities,
   * if any.
   *
   * This function is most useful on asynchronous actions which require thread-shifting back
   * onto some other thread pool (e.g. off of an event dispatch thread).
   */
  final def shiftSuffix(implicit EC: ExecutionContext): IO[A] = {
    attempt.flatMap { e =>
      IO async { (cb: Either[Throwable, A] => Unit) =>
        EC.execute(new Runnable {
          def run() = cb(e)
        })
      }
    }
  }

from cats-effect.

Closing this now since I've implemented all the things. Please feel free to open more for other desired changes!

from cats-effect.