search

mcollina / hyperemitter

Horizontally Scalable EventEmitter powered by a Merkle DAG
ISC License
70 stars 2 forks source link

Refactoring idea #17

Open mcollina opened 9 years ago

mcollina commented 9 years ago

So, I think we can refactor this in:

  1. hyperemitter-core, which is just the emitter, accepting a level in the constructor
  2. hypem, which is the CLI tool
  3. hyp2p, which is the client/server logic

Where HyperEmitter just bundles all of them up.

@mcdonnelldean @asbjornenge what do you think?

asbjornenge commented 9 years ago

Sounds reasonable to me :+1: I would be interested in looking at and perhaps reusing the hyp2p logic, and also core for some other ideas that are forming. :smile:

mcdonnelldean commented 9 years ago

@mcollina Do you mean as three (4) modules or as one module in 3 pieces? I was looking into this very thing yesterday, trying to devise the best way to split it.

Are we saying that all of the P2P stuff ends up in the client / server logic?

mcollina commented 9 years ago

Are we saying that all of the P2P stuff ends up in the client / server logic?

That's my proposal, yes.

mcdonnelldean commented 9 years ago

@mcollina and the modules question?

Let me look at this tonight, I might do up a very rough PR, the part I am struggling with is who owns the auto connect to peer logic. Looking at it I'm thinking the peer should have the ability to connect to other peers, receive connections and then set up pump streams, from and to the peer.

If that's true the emitter is going to get really small, essentially managing streams and codecs; handing off the rest to the peer.

mcollina commented 9 years ago

I mean 4 different modules on NPM. This approach has proved itself worth pursuing.

The main reason for splitting the P2P stuff, is that I would like to test it synchronously, or at least in a vacuum. Possibly it should be independent from hyperemitter. The core reason is that there are tons of network failures that are impossible to reproduce using sockets, but I would like to be able to test them in the tests.

Basically it should be a) protocol generic b) offer a simple API for connecting two peers c) offer an API to be notified of new peers that are available d) offer an API to signal that a peer has been disconnected. Then we can add smart policies, so that 'connect only to N/2 + 1 peers' or something similar. Does all this makes any sense?

mcdonnelldean commented 9 years ago

We are in agreement so.

To drill down here: It should be notified of new peers, it shouldn't manage this itself? So if hyperemitter was using this lib, it would connect and pass a collection of known peers to it.

mcollina commented 9 years ago

Yep. In my experience keeping the two concern separated is really good: discovery and connection handling should be handled by two different components.

mcdonnelldean commented 9 years ago

Cool, ok, I'll crack on with an example, I'll pop it in /examples for now and then when it is fleshed out more move it to it's own module. Any ideas what you want to call the cli tool? hyperemitter-cli?

mcollina commented 9 years ago

how about just hypem?

Il giorno lun 15 giu 2015 alle ore 15:06 Dean McDonnell < notifications@github.com> ha scritto:

Cool, ok, I'll crack on with an example, I'll pop it in /examples for now and then when it is fleshed out more move it to it's own module. Any ideas what you want to call the cli tool? hyperemitter-cli?

— Reply to this email directly or view it on GitHub https://github.com/mcollina/hyperemitter/issues/17#issuecomment-112063509 .

mcdonnelldean commented 9 years ago

I'm good with that too :dancer: