Skip to content
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

Add a Future companion method like Future.sequence that return Successes only. #9295

Open
scabug opened this issue May 3, 2015 · 7 comments

Comments

@scabug
Copy link

scabug commented May 3, 2015

Future.sequence is failing when one of the Future is failed.
This is very useful but sometimes you want to ignore the failed futures and just get the list of successful futures.

The method to filter the successes only can be easily written :

 def sequenceSuccess[A, M[X] <: TraversableOnce[X]](in: M[Future[A]])(implicit cbf: CanBuildFrom[M[Future[A]], A, M[A]], executor: ExecutionContext) = {
  	in.foldLeft((Future.successful(cbf(in)))) {
      (fr, fa) =>
      (for (r <- fr; a <- fa) yield (r += a)) recoverWith {
      	case _ => fr
      }
    } map (_.result())
    }  

An extension of this feature would be another method that returns the following Tuple

(successes : Future[Seq[A]], failures : Future[Seq[Throwable]])
@scabug
Copy link
Author

scabug commented May 3, 2015

Imported From: https://issues.scala-lang.org/browse/SI-9295?orig=1
Reporter: Gaetan Hervouet (gramk)

@saeltz
Copy link

saeltz commented Jun 10, 2020

@viktorklang, is this something that has a chance to make it into the core standard library?

We needed such a method for some production code yesterday and didn't find a good solution and in the end wrapped our results in an Option[A] like the following:

.transformWith {
        case Failure(_) => Future.successful(None)
        case Success(value: A) => Future.successful(Some(value))
      }

and then traversed and collected where _.isDefined. That's obviously not beautiful.

This definitely interests some more people, see:
https://stackoverflow.com/questions/20874186/scala-listfuture-to-futurelist-disregarding-failed-futures

I'd be willing to spend some time on this.

@SethTisue SethTisue added this to the Backlog milestone Jun 10, 2020
@viktorklang
Copy link

viktorklang commented Jun 10, 2020

@saeltz

I've seen this pop up a couple of times, so it's clear that there is some need for equivalent functionality. Not sure if something specialized such as sequenceSuccesses is the right solution though. Btw, your code can be written as:

.transform(t => Success(t.toOption))

Perhaps an alternative would be:

final def sequenceSelectively[A, CC[X] <: IterableOnce[X], To](in: CC[scala.concurrent.Future[A]], predicate: Try[A] => Boolean)(implicit bf: scala.collection.BuildFrom[CC[scala.concurrent.Future[A]],A,To], implicit executor: scala.concurrent.ExecutionContext): scala.concurrent.Future[To]

where sequence would then become:

final def sequenceSelectively[A, CC[X] <: IterableOnce[X], To](in: CC[scala.concurrent.Future[A]])(implicit bf: scala.collection.BuildFrom[CC[scala.concurrent.Future[A]],A,To], implicit executor: scala.concurrent.ExecutionContext): scala.concurrent.Future[To] = sequenceSelectively(in, {
  case Success(_) => true
  case Failure(t) => throw t
})

Haven't thought about it enough, but I'd rather create something more flexible, than special-casin g a use-case.

@saeltz
Copy link

saeltz commented Jun 10, 2020

.transform(t => Success(t.toOption))

That looks already much better. Thanks!

I'd be fine with sequenceSelectively. Should I try to implement it or what would be the next step?

@SethTisue
Copy link
Member

Should I try to implement it or what would be the next step?

It's going to be a while before anything can be added to the standard library; see scala/scala-dev#661

@saeltz
Copy link

saeltz commented Jun 13, 2020

@SethTisue, thanks for the info! Would I have to open a new PR in scala-collections-compat for this to be considered? There's no hurry though.

@viktorklang, please see scala/scala@fd23d4a for a first implementation. I'd be happy for any feedback!

I tested using

import scala.concurrent.{ExecutionContext, Future}
import scala.util.{Try, Success, Failure}
import ExecutionContext.Implicits._

object Test extends App {
  val f1 = Future.successful(1)
  val f2 = Future.successful(2)

  // both sequence methods return the same successful Future[Seq[A]]
  val res1 = Future.sequence(f1 :: f2 :: Nil)
  res1 foreach println
  val res2 = Future.sequenceOld(f1 :: f2 :: Nil)
  res2 foreach println

  val f3 = Future.failed(new RuntimeException)

  // both sequence methods return the same Future.failed
  val res3 = Future.sequence(f1 :: f3 :: Nil)
  res3 onComplete {
    case Success(value) => println(value)
    case Failure(exception) => println(exception)
  }
  val res4 = Future.sequenceOld(f1 :: f3 :: Nil)
  res4 onComplete {
    case Success(value) => println(value)
    case Failure(exception) => println(exception)
  }
  res4 foreach println

  // we can select only successes with the new method
  val selectSuccesses: Try[Int] => Boolean = {
    case Success(_) => true
    case Failure(_) => false
  }
  val res5 = Future.sequenceSelectively(f1 :: f2 :: f3 :: Nil, selectSuccesses)
  res5 foreach println
}

@SethTisue
Copy link
Member

Would I have to open a new PR in scala-collections-compat for this to be considered?

I'm afraid we're not ready yet; scala/scala-collection-compat#328 and scala/scala-collection-compat#329 are still open/in-progress

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants