-
Notifications
You must be signed in to change notification settings - Fork 533
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
Fix csot test failures #2853
Fix csot test failures #2853
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, I'm really happy to see the switch to IO.select
versus Timeout.timeout
. I'm not adamant about the duplicated code--I can imagine how it might be a little tricky to refactor that out, and if it is, then don't worry about it.
Otherwise, LGTM! 👍
else | ||
retry | ||
end | ||
retry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a stray retry
here. Also, feels like these two rescue
blocks have a lot in common (differ only on whether the sockets are waiting for read or write). Could the logic be pulled out somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, thank you. Extracted the retry logic in a separate method.
No description provided.