bpot / poseidon

A client for Kafka 0.8
MIT License
260 stars 101 forks source link

Nonblocking socket initialisation #79

Open szymonsobczak opened 9 years ago

szymonsobczak commented 9 years ago

This fixes #78.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.94%) when pulling 08c85fb1c5789527d5c9ade8c12a163affdf6bdd on futuresimple:nonblocking_socket into b4dc2bec3c2c5b8957a821083602981b329f8bb6 on bpot:master.

bpot commented 9 years ago

This looks great! I would like to have an integration test that reproduces the issue. Have you looked at writing one?

It would probably have to be linux only (b/c of iptables) but I think that's fine.

szymonsobczak commented 9 years ago

Hi @bpot,

I've added the integration test. It requires

The spec fails when connection is using TCPSocket.new() (in my case it takes >30s to timeout) and passes with connect_with_timeout().

Some minor fixes were required to run the suite on OSX, like running OS commands differently and ensuring required dirs exist.

I've also worked a bit with sleeps, as the tests were unnecessarily slow and hard to work with. There is still room for improvement - most of the actions can be detected from watching kafka logs, so there should be no need for sleeps whatsoever.

BTW, we're using your library on production already for a couple months now and it looks stable, so maybe it's time to release v1.0.0?

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.43%) to 91.51% when pulling b67a106499c5d2a61a977fb0b68be3ab8996f9ac on futuresimple:nonblocking_socket into b4dc2bec3c2c5b8957a821083602981b329f8bb6 on bpot:master.

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-0.84%) to 91.1% when pulling a6877d18a86c8aca86d92cbf3051101ee7b08017 on futuresimple:nonblocking_socket into b4dc2bec3c2c5b8957a821083602981b329f8bb6 on bpot:master.

mensfeld commented 9 years ago

is this going to be merged?