docker / libchan

Like Go channels over the network
Apache License 2.0
2.47k stars 142 forks source link

Streamchan implementation #93

Closed dmcgowan closed 8 years ago

dmcgowan commented 9 years ago

The network implementation is not transport agnostic. Netchan makes use of the stream provider and encoding interface.

This PR may be split up and dependent on other PRs for new msgpack implementation, protocol updates, and encoding interface. Created this PR now to being reviewing interface.

Depends on #85

tiborvass commented 9 years ago

@dmcgowan I must say though that I don't like the name netchan as it has connotation with Rob Pike's version. I would be more okay with it, if it provided a channel interface like Rob Pike's, but if not, then I would try to name it something else.

dmcgowan commented 9 years ago

I actually feel the same way, but required the least thought. We can discuss a better name here. Other thought was streamchan, but sounds ugly. Can also be decided once the interface is solidified, something else may make sense then.

tiborvass commented 9 years ago

No problem, and sorry to start with the "name-bikeshedding" on your awesome PR! I just thought it was important to make sure it's dealt with later. I personally think that, unless it is super clear that we are not talking about Go channels, we should avoid the term Channel. Maybe simply stream ? Anyway let's postpone the naming debate :)

dmcgowan commented 9 years ago

I have no problem with "name-bikeshedding" since naming is always the most important and usually the best one only presents itself after many names have been proposed. The main function in the interface is NewTransport, stream.NewTransport could work. The current netchan directory is mostly tests and only 350ish lines of code now, I should probably do a better job defining exactly what those lines of code do.

stevvooe commented 9 years ago

The abstraction here might be a little higher than I would have gone with but this looks fine.

Something like this, perhaps:

func NewMultiplexer(io.ReadWriter) (Multiplexer, error)

type Multiplexer interface {
    // Channel returns a channel associated with the id, allocating 
    // the channel if it does not yet exist.
    Channel(id int) (io.ReadWriteCloser, error)
}

Then, streams, encoders and decoders pull channels off the multiplexed stream. The base NewMultiplexer can be a libchan specific framing protocol but other implementations could provide Multiplexers.

dmcgowan commented 9 years ago

The multiplexing abstraction layer is one I have given quite a bit of thought to and moved out to here

https://github.com/dmcgowan/streams

Feedback on streams would be appreciated as it is intended to have uses larger than libchan.

mcollina commented 9 years ago

:+1: for the separated streams library.

dmcgowan commented 8 years ago

Closing for now, going to split up the PRs to make it easier to review