finagle-websocket icon indicating copy to clipboard operation
finagle-websocket copied to clipboard

Added an example with an echo server and a console client

Open mkhq opened this issue 8 years ago • 5 comments

Started on a new examples sub-project which can be used to showcase how to use finagle-websocket and as a way to try out the new API. If this does not belong in the main repo, please let me know.

This commit contains a console client that reads a line of text from stdin and sends it to an echo server. The server uses a ServiceFactory to keep track of websocket sessions.

I have a couple of points on the API:

  • It took some time to get used to AsyncStream, but after some trial and error I think it is better compared to the Offer/Broker approach.
  • When creating the client side Request, the SocketAddress is either set to null (from the other examples) or a new SocketAddress {}. Could we swap the input argument and provide a default value instead since this is more interesting to have access to on the server side?
  • In the example, the ServiceFactory is used to get access to the ClientConnection for reporting on when the connection closes. Is there another way of doing this or is it recommended to use a ServiceFactory? An alternative would be to extend the request with an onClose promise as was done before, what are the trade-offs for doing so?

mkhq avatar May 11 '16 22:05 mkhq

Coverage Status

Coverage remained the same at 73.529% when pulling b8305a690f51ca40ec3d4f4dbe638b2db9d909d4 on mkhq:examples-asyncstream-client-server into 34d4bd81ad1d3a5f866b1c01bd61e2d785f4947d on finagle:develop.

coveralls avatar May 11 '16 22:05 coveralls

Coverage Status

Coverage remained the same at 73.529% when pulling 43c927bcc5d1c7883154e9eb445abaaaf39b11f9 on mkhq:examples-asyncstream-client-server into 34d4bd81ad1d3a5f866b1c01bd61e2d785f4947d on finagle:develop.

coveralls avatar May 16 '16 14:05 coveralls

Thanks @mkhq! I think it's nice to have examples, but we'll see what @sprsquish thinks.

I want to echo your thoughts about the SocketAddress on the client request. It doesn't really make sense, and I'm not happy with that API either. I have some ideas, but I want to flesh them out a little before presenting them.

The end of the AsyncStream means the same thing as connection closure. Because the stream is persistent, you can do something like this in the Service handler.

request.messages.foreach(_ => ()).ensure { println("Connection closing") }

luciferous avatar Jun 09 '16 23:06 luciferous

Hey guys, sorry it's been so long. I'm adding @luciferous as an admin collaborator to the repo, @mkhq you're already on the team. I lack sufficient time and motivation to continue this project.

Thanks for picking up the torch so far and bringing the library into the present.

sprsquish avatar Jul 10 '16 14:07 sprsquish

@sprsquish Thanks! I'll do my best to attend to these issues regularly. We'll see what happens.

luciferous avatar Jul 22 '16 04:07 luciferous