Client Definitions based on free algebras - Unary Services #16

juanpedromoreno · 2017-06-28T13:11:05Z

This PR solves partially the issue #11 (only unary services).

It brings some free algebras on top of Grpc for RPC clients. Concretely, it solves:

Channel Configuration.
Channel Algebra to be able to create new RPC calls, based on method descriptors and some call options.
ClientCalls Algebra to perform the different types of calls (unary, client streaming, server streaming and bidirecctional).

It also updates the demo code to use these algebras, composing RPC services as freestyle modules:

@module
trait ClientAPP {
  val greetingClientM: GreetingClientM
  val echoClientM: EchoClientM
}

object GreetingClientApp {

  val messageRequest = MessageRequest("Freestyle")
  val echoRequest    = EchoRequest("echo...")

  def unaryDemo[F[_]](implicit APP: ClientAPP[F]): FreeS[F, (String, String, String)] = {

    val greetingClientM: GreetingClientM[F] = APP.greetingClientM
    val echoClientM: EchoClientM[F]         = APP.echoClientM

    val defaultOptions = CallOptions.DEFAULT

    for {
      hi   <- greetingClientM.sayHello(messageRequest, defaultOptions)
      bye  <- greetingClientM.sayGoodbye(messageRequest, defaultOptions)
      echo <- echoClientM.echo(echoRequest, defaultOptions)
    } yield {
      println(s"Received -> (${hi.message}, ${bye.message}, ${echo.message})")
      (hi.message, bye.message, echo.message)
    }
  }

  def runProgram[F[_]](implicit M: Monad[F]) = {

    Await.result(unaryDemo[ClientAPP.Op].interpret[Future], Duration.Inf)
  }
}

codecov-io · 2017-06-28T13:24:31Z

Codecov Report

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

@@         Coverage Diff          @@
##           master   #16   +/-   ##
====================================
  Coverage       0%    0%           
====================================
  Files          10    14    +4     
  Lines         130   154   +24     
  Branches        3     2    -1     
====================================
- Misses        130   154   +24

Impacted Files	Coverage Δ
rpc/src/main/scala/server/package.scala	`0% <ø> (ø)`	⬆️
...eting/src/main/scala/greeting/GreetingClient.scala	`0% <ø> (ø)`	⬆️
...ng/src/main/scala/greeting/GreetingServerApp.scala	`0% <ø> (ø)`	⬆️
rpc/src/main/scala/server/implicits.scala	`0% <ø> (ø)`	⬆️
rpc/src/main/scala/client/package.scala	`0% <0%> (ø)`
...c/main/scala/client/handlers/ChannelMHandler.scala	`0% <0%> (ø)`
rpc/src/main/scala/client/implicits.scala	`0% <0%> (ø)`
...ng/src/main/scala/greeting/GreetingClientApp.scala	`0% <0%> (ø)`	⬆️
...in/scala/client/handlers/ClientCallsMHandler.scala	`0% <0%> (ø)`
...ng/src/main/scala/greeting/runtime/implicits.scala	`0% <0%> (ø)`	⬆️
... and 4 more

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 f9b3fad...362b8f5. Read the comment docs.

raulraja

Amazing work @juanpedromoreno, Left minor comments that I'd like addressing before merging this but none of them blocking. Great stuff 👍

raulraja · 2017-06-28T13:34:47Z

demo/greeting/src/main/scala/greeting/GreetingClientApp.scala

+    val defaultOptions = CallOptions.DEFAULT
+
+    for {
+      hi   <- greetingClientM.sayHello(messageRequest, defaultOptions)


Since these operations are not interdependent this could be better expressed in applicative style.

(a |@| b |@| c).map( ... )

raulraja · 2017-06-28T13:35:42Z

demo/greeting/src/main/scala/greeting/GreetingClientApp.scala


-    // http://www.grpc.io/docs/guides/concepts.html
+  def runProgram[F[_]](implicit M: Monad[F]) = {


Do we need a monad constrain here?

Yes, it's required. Not sure if we could skip it somehow

raulraja · 2017-06-28T13:37:31Z

demo/greeting/src/main/scala/greeting/client/GreetingClientM.scala

+import io.grpc.stub.StreamObserver
+
+@module
+trait GreetingClientM {


raulraja · 2017-06-28T13:40:00Z

demo/greeting/src/main/scala/greeting/runtime/implicits.scala

+
+}
+
+object implicits {


I think users may be confused with this. Every time you import something with the name implicits users may go for implicits._ right away. Since this is a demo is not critical but I think we should try avoiding nesting further implicit scopes once we hit the implicits one. For this case I would just have:
object implicits extends ServerImplicits with ClientImplicits

raulraja · 2017-06-28T13:45:57Z

rpc/src/main/scala/client/handlers/ClientCallsMHandler.scala

+
+import scala.collection.JavaConverters._
+
+class ClientCallsMHandler[F[_]](implicit C: Capture[F], AC: AsyncContext[F])


We have traditionally been parametrizing handlers with M[_] instead of F[_] since in other classes aggregating algebras F[_] we have used it to refer to the coproduct. I'm fine if we call this F but in that case we should do this across all modules. Also it would be nice if we all as a team come up with a naming convention doc that we enforce across the entire org to make sure we are consistent and not confusing users.

raulraja · 2017-06-28T13:48:00Z

rpc/src/main/scala/client/handlers/ClientCallsMHandler.scala

+class ClientCallsMHandler[F[_]](implicit C: Capture[F], AC: AsyncContext[F])
+    extends ClientCallsM.Handler[F] {
+
+  def asyncUnaryCall[I, O](


Do we actually want to call this things unary etc...? Perhaps just async, sync and stream would be enough. thoughts?

Sounds good, as we have to differentiate internally each service (and @free doesn't support method overloading), these are the new names: async, asyncStreamServer, asyncStreamClient, asyncStreamBidi, sync, syncC, syncStreamServer, syncStreamServerC, asyncM

raulraja · 2017-06-28T13:49:55Z

rpc/src/main/scala/client/package.scala

+            case SetCompressorRegistry(registry)    => acc.compressorRegistry(registry)
+            case SetIdleTimeout(value, unit)        => acc.idleTimeout(value, unit)
+            case SetMaxInboundMessageSize(max)      => acc.maxInboundMessageSize(max)
+          }).asInstanceOf[ManagedChannelBuilder[_]]


any way we can get rid of this cast?

raulraja · 2017-06-28T13:51:24Z

rpc/src/main/scala/server/implicits.scala

@@ -29,8 +29,8 @@ trait Syntax {

  final class ServerOps(server: FreeS[GrpcServer.Op, Unit]) {

-    def bootstrapM[M[_]](implicit MM: Monad[M], handler: GrpcServer.Op ~> M): M[Unit] =
-      server.interpret[M]
+    def bootstrapM[F[_]](implicit M: Monad[F], handler: GrpcServer.Op ~> F): F[Unit] =


Regarding the naming conventions mentioned earlier. should this just be M[_]: Monad instead of an implicit arg that is not in use in the method body?

Client definitions based on free algebras

6faf553

juanpedromoreno mentioned this pull request Jun 28, 2017

3. Client Definitions #11

Closed

5 tasks

raulraja approved these changes Jun 28, 2017

View reviewed changes

juanpedromoreno added 2 commits June 28, 2017 17:05

Addresses code review comments

3629a5b

Removes the superfluous Monad constraint

362b8f5

juanpedromoreno merged commit a142c5f into master Jun 28, 2017

juanpedromoreno deleted the feature/client-definitions branch June 28, 2017 16:00

SemanticBeeng mentioned this pull request Oct 31, 2018

MTL style behaviour algebra notxcain/aecor#50

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Client Definitions based on free algebras - Unary Services #16

Client Definitions based on free algebras - Unary Services #16

juanpedromoreno commented Jun 28, 2017 •

edited

Loading

codecov-io commented Jun 28, 2017 •

edited

Loading

raulraja left a comment

raulraja Jun 28, 2017

raulraja Jun 28, 2017

juanpedromoreno Jun 28, 2017

raulraja Jun 28, 2017

raulraja Jun 28, 2017

raulraja Jun 28, 2017

raulraja Jun 28, 2017

juanpedromoreno Jun 28, 2017 •

edited

Loading

raulraja Jun 28, 2017

juanpedromoreno Jun 28, 2017

raulraja Jun 28, 2017


		// http://www.grpc.io/docs/guides/concepts.html
		def runProgram[F[_]](implicit M: Monad[F]) = {


		import scala.collection.JavaConverters._

		class ClientCallsMHandler[F[_]](implicit C: Capture[F], AC: AsyncContext[F])

Client Definitions based on free algebras - Unary Services #16

Client Definitions based on free algebras - Unary Services #16

Conversation

juanpedromoreno commented Jun 28, 2017 • edited Loading

codecov-io commented Jun 28, 2017 • edited Loading

Codecov Report

raulraja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanpedromoreno Jun 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juanpedromoreno commented Jun 28, 2017 •

edited

Loading

codecov-io commented Jun 28, 2017 •

edited

Loading

juanpedromoreno Jun 28, 2017 •

edited

Loading