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

Support for Scala Future's #249

Closed
schmitch opened this Issue Sep 17, 2015 · 3 comments

Comments

3 participants
@schmitch
Contributor

schmitch commented Sep 17, 2015

Hello, currently it would be great if scala also has support for scala native future's.
Currently If I do the following

import com.twitter.finagle.httpx.{Response, Status, Request}
import com.twitter.finatra.http.Controller
import scala.concurrent.{ExecutionContext, Future}

class HelloWorldController extends Controller {
  import ExecutionContext.Implicits.global

  get("/hi") { request: Request =>

    val r = Response(Status.Ok)
    r.setContentString("Hello")
    Future(r) // or Future.successful(r)
  }
}

I will get the future.toString instead of the content of the file (scala.concurrent.impl.Promise$DefaultPromise@b2082d3), which is really aweful, especially when you want to use finatra with other scala code which using scala.concurrent.Future's

@mattweyant

This comment has been minimized.

mattweyant commented Sep 17, 2015

Twitter's bijection library has methods for converting between the two. Does that help?

@schmitch

This comment has been minimized.

Contributor

schmitch commented Sep 18, 2015

Not that much, native support would be way better, however in the near future this will be easier if finatra would implement the Java interfaces that scala.concurrent.Future's will use in higher scala versions.

@cacoco

This comment has been minimized.

Member

cacoco commented Sep 19, 2015

Finatra is built on top of Twitter's Finagle and therefore uses Twitter's com.twitter.util.Future. As such, we have no plans at this time to move to natively supporting Scala's Future (introduced in Scala 2.10). There may be a more holistic movement to support this in Twitter's stack in the future but for now targeted use of the bijection library is probably the way to go when/if you need to convert between the two Futures.

Also in your code example, there does not seem to be a reason for you to use a Future (of either type) here and you are not required to wrap your responses in a Future. See this section on Responses in Finatra for more information. For your example you could just use the ResponseBuilder.

E.g.,:

import com.twitter.finagle.httpx.{Response, Status, Request}
import com.twitter.finatra.http.Controller
import com.twitter.finatra.http.response.ResponseBuilder

class HelloWorldController @Inject() (
  response: ResponseBuilder) extends Controller {

  get("/hi") { request: Request =>
   response.ok("Hello")
  }
}

Thanks!

@cacoco cacoco closed this Sep 19, 2015

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