Well Rested Futures: Implement Scala Future support in RestHelper#1741
Conversation
It's really a general abstraction, and we'll want to reuse it for RestHelper.
This should allow RestHelper to auto-wrap Scala Future responses into RestHelper.async handling, in addition to the LAFuture support that already existed. Also, the RestHelper.async thread will no longer block while waiting for its future (Lift or Scala or whatever) to resolve.
There was a problem hiding this comment.
What makes an implicit low or high priority?
There was a problem hiding this comment.
Where it is in the resolution order; these, however, are misnomers. They'd be low priority if they were in a trait mixed into the singleton. Need to fix that.
There was a problem hiding this comment.
Made them actually low priority.
Shadowfiend
force-pushed
the
well-rested-futures
branch
from
0dee811 to
d0f16ce
Compare
We test it with an LAFuture, but the point is any CanResolveAsync will do.
|
This should be good for another 👀. |
|
Code looks good but it looks like your spec fails in Travis. https://travis-ci.org/lift/framework/builds/96416723#L2972-L2977 |
|
I'm thinking probably the |
We were using that to fail if the spec hadn't sastified its result future with the value received from the async resolution, but it looks like those satisfactions happen async, so we can't be sure that the future will be immediately complete once we hit that line of code. Instead we just let `get` block until we get the proper resolution, which should still be very quick.
|
Looks like the remaining codacy issues are related to duplication and “complexity”, but I don't see how we can improve on those, so I think this should be good to merge. |
|
Lehgo! |
RestHelper has long supported returning an LAFuture as the result of a RestHelper block and auto-wrapped those in RestHelper.async so they're dealt with via continuation, much like comet responses. This PR extends RestHelper to support the same functionality for Scala Futures, thus rounding out our dual support for Scala and Lift futures both here and in snippet bindings.
3 participants
RestHelperhas long supported returning anLAFutureas the result ofa
RestHelperblock and auto-wrapped those inRestHelper.asyncsothey're dealt with via continuation, much like comet responses. This PR
extends
RestHelperto support the same functionality for ScalaFutures,thus rounding out our dual support for Scala and Lift futures both here and
in snippet bindings.
We do this by moving the
CanResolveAsyncabstraction introduced forsnippet async future binding into
lift-utiland then reusing that abstractionin
RestHelper. We also add some specs forCanResolveAsyncand aspec to prove that
LAFutureis still working as expected.Closes #1735.