Eyepea / aiosip

SIP support for AsyncIO (DEPRECATED)
Apache License 2.0
82 stars 41 forks source link

[peers] make Peer.close() async, actually use connection_lost #61

Closed jlaine closed 6 years ago

jlaine commented 6 years ago

The previous closing mechanism:

The new mechanism is clearer as we clearly distinguish "we want to close the transport" and "the transport has closed". It also allows for clients and servers to shutdown cleanly regardless of each peer's transport.

We also stop pretending a Peer can be used as a context manager.

codecov-io commented 6 years ago

Codecov Report

Merging #61 into master will increase coverage by 0.22%. The diff coverage is 96.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
+ Coverage   71.22%   71.44%   +0.22%     
==========================================
  Files          15       15              
  Lines        1449     1443       -6     
  Branches      259      260       +1     
==========================================
- Hits         1032     1031       -1     
+ Misses        309      307       -2     
+ Partials      108      105       -3
Impacted Files Coverage Δ
aiosip/peers.py 69.4% <100%> (+1.15%) :arrow_up:
aiosip/protocol.py 62.37% <80%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c7c29a5...50a69e6. Read the comment docs.

ovv commented 6 years ago

I need to take some time to go back into the code, it's been a few month since I last played with it. I'll try to do that next week. Hopefully then things will be clearer

ovv commented 6 years ago

@vodik I haven't had time to look into it yet but if you want feel free to merge this.

vodik commented 6 years ago

introduced an unfortunate coupling between Peer and Connector, where the peer needed to know about it's connector

I'm pretty confident we can overcome this with some more refactoring, which will have to happen anyways since we should support UDP+TCP.

So nice work. Merging.