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

New method for WebSocketBuilder taking Pipe[F, WebSocketFrame, WebSocketFrame] #1863

Closed
asoltysik opened this issue May 15, 2018 · 5 comments
Labels
RFC Design ideas that we'd like to spur discussion

Comments

@asoltysik
Copy link

What's your opinion about another build method that looks like this:

 def build(
        pipe: Pipe[F, WebSocketFrame, WebSocketFrame]
        headers: Headers = Headers.empty,
        onNonWebSocketRequest: F[Response[F]] =
          Response[F](Status.NotImplemented).withEntity("This is a WebSocket route.").pure[F],
        onHandshakeFailure: F[Response[F]] = Response[F](Status.BadRequest)
          .withEntity("WebSocket handshake failed.").pure[F])

Similarly how it's done in akka-http. I believe it is enough for many use cases of websockets, can simplify the code and it's easier to reason about for these use cases.

@rossabaker rossabaker added the RFC Design ideas that we'd like to spur discussion label May 16, 2018
@rossabaker
Copy link
Member

As an additional builder or as the only builder?

What does it mean? The input to the pipe are received frames and the output is sent frames?

@SystemFw
Copy link
Member

From what I could get, it would be an additional builder, and yes, a Pipe from input to output

@SystemFw
Copy link
Member

I'm actually thinking that a model based on a Pipe might be cleaner than what we have now, here's a small snippet discussing translating back and forth between the two:

case class Builder(pipe: Pipe[F, Frame, Frame])
case class Split(in: Sink[F, Frame], out: Stream[F, Frame])

def splitToBuilder(s: Split) = Builder(input =>  s.out.concurrently(input.to(s.in))
case class Builder(in: Sink[F, Frame], out: Stream[F, Frame])
case class Piped(pipe: Pipe[F, Frame, Frame])

def pipedToBuilder(p: Piped): F[Builder] = 
needs to create the queue, and also quite possibly wrap everything in Option to use `unNoneTerminate`, 
without it you get race conditions like the one in Chris example that requires `sleep`

@jmcardon
Copy link
Contributor

p.s I am 👍 on this, in that reacting to messages is often what you want

@rossabaker
Copy link
Member

This exists now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Design ideas that we'd like to spur discussion
Projects
None yet
Development

No branches or pull requests

4 participants