Polyconseil / aioamqp

AMQP implementation using asyncio
Other
280 stars 88 forks source link

Using transport to close connection #41

Closed mwfrojdman closed 9 years ago

mwfrojdman commented 9 years ago

It seems aioamqp.connect returns both the transport and the protocol objects now. And the docs at http://aioamqp.readthedocs.org/en/latest/api.html#starting-a-connection instruct to close the connection using yield from transport.close().

Is returning the transport to the library's user neat? It seems to make things more complicated than necessary as AmqpProtocol also has a close method.

dzen commented 9 years ago

Hello @mwfrojdman,

I think the readthedoc is not up to date. I updated the doc recently to match the doc: https://github.com/Polyconseil/aioamqp/blob/master/docs/api.rst.

You're right, closing the transport literally means "close the undelaying socket". Maybe the protocol should do this.

mwfrojdman commented 9 years ago

Oh that's good. I think the current behaviour in AmqpProtocol is fine, as it sends a close connection message to the server, and the server closes the socket. Which will lead to a TCP packet being sent to the client informing about the thing, so there should be no extra delay on the client side either compared to the client closing the socket. Also, if the client sends the close connection message to the server and continues to immediately close the socket, it's not guaranteed that the message will ever go out. TLDR: I think it's good as it is.

dzen commented 9 years ago

I also forced readthedocs to update the documentation: http://aioamqp.readthedocs.org/en/master/api.html