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

Proposal: Factor out connection logic #125

Open
msiebuhr opened this issue Sep 26, 2019 · 3 comments
Open

Proposal: Factor out connection logic #125

msiebuhr opened this issue Sep 26, 2019 · 3 comments

Comments

@msiebuhr
Copy link
Contributor

I've previously written another statsd-client, where I ended up factoring out the connection-handling into a separate file per connection type:

  • Common-ish interface to a "connection" makes the core client much simpler.
  • Easier to test when can poke at more "internals" in the connection logic.
  • Lowers the bar for adding new kinds of connections.

ATM, I see UDP, UDS, TCP and mock connection types, which all seem to have quirks and special-casing to some degree, and I think a split-up like this would simplify things quite a bit.

I don't personally have the bandwidth to take it on, but if someone is looking for something to hack on, I think this would be nice to work on (there's already a quite comprehensive test-suite to work with).

@bdeitte
Copy link
Collaborator

bdeitte commented Sep 28, 2019

@msiebuhr Thank you for the suggestion, and for your recent improvements here! Happy to have your help here, especially with your statsd library expertise, whenever you can give it.

@bdeitte
Copy link
Collaborator

bdeitte commented Oct 15, 2019

Looks like this did what you were thinking? https://github.com/brightcove/hot-shots/pull/127/files If so I'll close it out

@msiebuhr
Copy link
Contributor Author

I imagined having a udp.js, uds.js, tcp.js etc. would provide an obvious split. transport.js or whatever else would then pick the right one and maybe do some thin wrapping...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants