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
Fixing ticket #1650 #219
The head ref may contain hidden characters: "wip-1650-Future-or-\u221A"
Fixing ticket #1650 #219
Conversation
def register(to: Future[U]) = to onComplete { | ||
case r @ Right(_) ⇒ p tryComplete r | ||
case l @ Left(_) ⇒ that.value match { | ||
case Some(Left(_)) ⇒ p tryComplete l //If he failed, race for setting failure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like it 'that' fails before 'this' succeeds the promise will be completed with the failure. Is this intentional, or should register take both futures as args (instead of hardcoding 'that')?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! :-)
I've changed the semantics to cascade only failures. |
val p = Promise[U]() | ||
onComplete { | ||
case r @ Right(_) ⇒ p complete r | ||
case _ ⇒ that onSuccess { case r ⇒ p success r } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it intentional that if that
also fails, p
is never completed? (I as a user would not expect that kind of behavior)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would you expect? The failure of the first, or of the second?
Can I merge now plz? |
u can haz me smiley: :) |
Removing the flawed orElse and replacing it with the more awesomized "or".