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

Provides grpc configuration DSL and GrpcServer algebras #13

Merged
merged 2 commits into from
Jun 26, 2017

Conversation

juanpedromoreno
Copy link
Member

@juanpedromoreno juanpedromoreno commented Jun 26, 2017

Server Demo has been also adapted to the newly introduced changes, where the RPC core provides helpers in order to bootstrap the server in the runtime monad, according to the server configuration.

This is a preliminary version towards to fulfill #10. We still need to find the way to aggregate all the services which would run in such a server.

For now, the user has to provide an implicit list with all the services the server will register:

https://github.com/frees-io/freestyle-rpc/compare/feature/server-definitions#diff-09bc282791d5598d8a70f248fbb125b0R33

Thanks @peterneyens for your suggestions related to the GrpcConfigInterpreter :)

Server bootstrap:

import cats.implicits._
import runtime.implicits._
import freestyle.rpc.server._
import freestyle.rpc.server.implicits._

import scala.concurrent.ExecutionContext.Implicits.global
import scala.concurrent.duration.Duration
import scala.concurrent.Await

object GreetingServerApp {

  def main(args: Array[String]): Unit =
    Await.result(server[GrpcServer.Op].bootstrapFuture, Duration.Inf)
}

(bootstrapFuture would be equivalent to bootstrap[Future])

Runtime dependencies:

import freestyle._
import freestyle.implicits._
import freestyle.rpc.demo.greeting._
import freestyle.rpc.server._
import freestyle.rpc.server.implicits._
import freestyle.rpc.server.handlers._

import scala.concurrent.{ExecutionContext, Future}

object implicits {

  implicit val config = Config(portNode1)

  implicit val grpcConfigs: List[GrpcConfig] = List(
    AddService(GreeterGrpc.bindService(new GreetingService, ExecutionContext.global))
  )

  implicit val grpcServerKleisli =
    new GrpcServerHandler[Future] andThen new GrpcConfigInterpreter[Future]

}

Server Demo has been also adapated to the new introduced changes, where the rpc core provides helpers in order to bootstrap the server in the runtime monad, according to the server configuration.
@codecov-io
Copy link

codecov-io commented Jun 26, 2017

Codecov Report

Merging #13 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@         Coverage Diff          @@
##           master   #13   +/-   ##
====================================
  Coverage       0%    0%           
====================================
  Files           6     9    +3     
  Lines         103   122   +19     
  Branches        2     2           
====================================
- Misses        103   122   +19
Impacted Files Coverage Δ
...ng/src/main/scala/greeting/GreetingClientApp.scala 0% <0%> (ø) ⬆️
rpc/src/main/scala/server/package.scala 0% <0%> (ø)
rpc/src/main/scala/server/implicits.scala 0% <0%> (ø)
...ng/src/main/scala/greeting/runtime/implicits.scala 0% <0%> (ø)
...main/scala/server/handlers/GrpcServerHandler.scala 0% <0%> (ø)
...ng/src/main/scala/greeting/GreetingServerApp.scala 0% <0%> (ø) ⬆️
demo/http/src/main/scala/user/package.scala 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eac2453...47930f9. Read the comment docs.

@juanpedromoreno juanpedromoreno force-pushed the feature/server-definitions branch 2 times, most recently from ee2fa00 to 4a8f0bf Compare June 26, 2017 18:12
Copy link
Contributor

@peterneyens peterneyens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments, but looks good in general (for someone with no grpc experience 😃).

import scala.concurrent.Future

trait FutureCaptureInstance {
implicit val `scala.concurrent.FutureCaptureInstance`: Capture[Future] = new Capture[Future] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like there is a Capture[Future] instance in the Capture companion object.

Kleisli(s => C.capture(s.start()))
}

def getPort: GrpcServerOps[F, Int] = Kleisli(s => C.capture(s.getPort))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can create a small helper method for this Kleisli(s => C.capture(s.foo)) pattern?

Something like

private def captureWithServer[A](f: Server => A] : GrpcServerOps[F, A] = Kleisli(s => C.capture(f(s))`

def getPort: GrpcServerOps[F, Int] = captureWithServer(_.getPort)


sealed trait GrpcConfig

case object GetDirectExecutor extends GrpcConfig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't look too closely at the grpc api, but GetDirectExecutor seems a bit strange for something which will never return something?


case class Config(port: Int)

sealed trait GrpcConfig
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably add extends Product with Serializable, otherwise the type of something like List(GetDirectExecutor) would be inferred as List[GrpcConfig with Product with Serializable].

@juanpedromoreno juanpedromoreno merged commit 57c1493 into master Jun 26, 2017
@juanpedromoreno juanpedromoreno deleted the feature/server-definitions branch June 26, 2017 21:35
Copy link
Contributor

@raulraja raulraja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff!

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

Successfully merging this pull request may close these issues.

4 participants