monix / monix-grpc

Apache License 2.0
6 stars 2 forks source link

Change the generic context parameter in the service #5

Closed borissmidt closed 3 years ago

borissmidt commented 3 years ago

Currently the user has to explicitly make a server for CTX type Metadata this also forces api caller to always pass a new Metadata() object.

Either we implement this with a typeclass like to keep the CTX support and give compilation errors for things that can't be passed as metadata initially. like

trait MetadataSerdes[T](){
  def fromMetadata(metadata: Metadata): Option[T]
  def toMetadata(msg: T): Metadata
  def default(): Metadata
}
object MetadataSerdes{
//a default for the Metadata as a pass through
}

//Using a trait will not be possible anymore :(
class MyGrpcServer[CTX](implicit ctx: MetadataSerdes[CTX]){
  def unary(request: MyRequest, myCall: CTX = ctx.default())
}

Or we just drop the CTX and use the vanilla Metadata and pass new Metadata() by default.

jvican commented 3 years ago

I think removing the metadata parameter here makes sense until we have a good story around how should users customize the metadata. One of the challenges of allowing users to do this is that they need to be exposed to the fact that metadata is mutable in the grpc-java API and can't be changed in certain places of the grpc pipeline. If this is a feature request that users want, we will need to figure out how to best accommodate the use case.

See https://github.com/fiadliel/fs2-grpc/issues/6 for the equivalent question in fs2-grpc. I think we should definitely not use a kleisli monad for this, we will need to find a more ergonomic solution to this, but it's nonetheless interesting to see it discussed in our sister library.

borissmidt commented 3 years ago

I would also skip that :) kelsi monad, i also think using meta data is in general not recomended since it cannot be described in the grpc proto defintion. So it is good for authentication and in the use of extra call handler wrappers.

So i agree for a quick authentication setting you use the metaData at the client side. But i think it is in general best avoided.