Skip to content

Conversation

asvetlov
Copy link
Member

@asvetlov asvetlov commented Dec 9, 2015

Fix for #448

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to make sure a socket was actually returned and the type of the socket is TCP.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

@asvetlov
Copy link
Member Author

Added set_tcp_nodelay method, more tests

@asvetlov
Copy link
Member Author

@fafhrd91 are you agree with PR?

@fafhrd91
Copy link
Member

i still think tcp_nodelay should be on by default. otherwise i am fine

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transports need not have a socket, i.e. get_extra_info('socket') might return a None.
So, should probably do a if sock and sock.family not in (socket.AF_INET, socket.AF_INET6) to be on the safe side.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asvetlov
Copy link
Member Author

Close in favor of #680
All comments are satisfied in the superseeder.

@asvetlov asvetlov closed this Dec 13, 2015
@asvetlov asvetlov deleted the connection_latency branch December 17, 2015 10:07
@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants