[Asterisk-code-review] sipp, test suite utils: Default media port to an unused port (testsuite[master])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/8691 )

Change subject: sipp, test_suite_utils: Default media port to an unused port
......................................................................


Patch Set 5:

(4 comments)

https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py
File lib/python/asterisk/utils_socket.py:

https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@222
PS5, Line 222:         empty list.
> Maybe I'm not completely understanding but I think it would make more sense
I think you're probably correct. I'll change it to throw an exception if not found (if port is 0 or otherwise).


https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@233
PS5, Line 233:             ports = self.get_avail(host, range(port[0] + 1, port[0] + span +
> I think replacing the argument/default 'span=0' with 'count=1' would make m
Okay will change it then. I've been allowing a negative span just in case someone ever wants the port and the 'span' ports just before port.

Still easy to change though. If someone happens to pass in 0 I'll just convert it to a 1.


https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@238
PS5, Line 238:         PortError(socktype, family, attempts)
> Yes I think we should throw an exception, sorry for the lack of python know
Oh haha my eyes totally missed that. You are indeed correct. It just creates the object but doesn't raise the exception.


https://gerrit.asterisk.org/#/c/8691/5/lib/python/asterisk/utils_socket.py@259
PS5, Line 259:         res = self.get_spanned(host, port, socktype, family, span, attempts)
> It may be good to check for an empty list here.
for the return? No need as the call to reserve can handle an empty list and then empty or not res would be the value we want returned.



-- 
To view, visit https://gerrit.asterisk.org/8691
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3da461123afc30e1f5ca12e65d289eaa42d6de00
Gerrit-Change-Number: 8691
Gerrit-PatchSet: 5
Gerrit-Owner: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Wed, 18 Apr 2018 23:22:01 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180418/20fc97a6/attachment.html>