Non-blocking Socket Connect #164
Conversation
|
Test failure due to issue documented in #166. |
|
I saw #167 first, but similar sentiment as my comment in #167 (comment) |
|
@jch @schaary does this change work on its own or should this introduce the changes as an option to
|
|
@mtodd I'd rather have this change not be an option so we can surface edge cases sooner. But I see you point about having an unstable API as well. I'll defer to what you feel is best. |
|
@jch well, we are working before a 1.0 so perhaps instability is warranted here. We've not been in the habit of performing pre-releases but in this case it might be worth the effort, assuming we can get integrators to test the prerelease in their environments. |
|
👍 |
| rescue Exception => e | ||
| # unexpected error | ||
| socket.close | ||
| raise Net::LDAP::LdapError, "Connection to #{host}:#{port} (#{ip}) failed: (#{e.class}) #{e.message}" |
There was a problem hiding this comment.
I think new exception class inheriting Net::LDAP::Error should be defined for this error.
|
If I'm not mistaken, #273 added non-blocking connections when timeout is set so I think we no longer need this. |
This replaces the existing, blocking connection implementation with non-blocking connection logic.
More (tests) to come.
cc @jch @schaary