<p> Attention is currently required from: Michael Bradeen. </p>
<p>Patch set 7:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/testsuite/+/18199">View Change</a></p><p>53 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199?tab=comments">Patch Set #7:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">I think this one patch needs to be broken into several patches based on the relevant changes done:</p><p style="white-space: pre-wrap; word-wrap: break-word;">1) A use SIPp 3pcc instead of pjsua patch<br>2) A patch for pjsua->pjsua2 changes<br>3) A python2.7/python3 compatibility patch<br>4) A debug/info extra logging patch</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File contrib/scripts/install_prereq:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/352bd7ed_3d71b6b1">Patch Set #7, Line 18:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">PACKAGES_DEBIAN="$PACKAGES_DEBIAN python3 python3-pip python3-setuptools python3-dev cython"<br># Testsuite: basic requirements:<br>PACKAGES_DEBIAN="$PACKAGES_DEBIAN python3-yaml python3-twisted-core python3-lxml liblua5.1-0-dev lua5.1 gdb"<br># Sipp requirements<br>PACKAGES_DEBIAN="$PACKAGES_DEBIAN libpcap-dev libssl-dev libsctp-dev"<br># pjsua requirements<br>#PACKAGES_DEBIAN="$PACKAGES_DEBIAN python3-dev"<br><br># Basic build system:<br>PACKAGES_RH="subversion git gcc gcc-c++ patch"<br># Python tools<br>PACKAGES_RH="$PACKAGES_RH python3 python3-pip python3-setuptools python3-devel Cython"<br># Testsuite: basic requirements:<br>PACKAGES_RH="$PACKAGES_RH PyYAML python3-twisted-core python3-lxml lua-devel gdb"<br># Sipp requirements<br>PACKAGES_RH="$PACKAGES_RH libpcap-devel openssl-devel lksctp-tools-devel"<br># pjsua requirements<br>#PACKAGES_RH="$PACKAGES_RH python3-devel"<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I wonder if we should have it check if python is already installed. If so install other libs based on the version, otherwise install python3 like you have here. Thoughts?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/astdicts.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/615df5c0_33bb9bc9">Patch Set #7, Line 274:</a> <code style="font-family:monospace,monospace"># print("__setitem__ key = ", key, " val = ", val)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Since this began as commented code I'd just remove it vs updating it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/abb2a976_7cde9dc7">Patch Set #7, Line 278:</a> <code style="font-family:monospace,monospace"># print("inserting key = ", key, " val = ", val)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same here. This line can be removed altogether.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/pcap.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/c7e088d7_6bb20008">Patch Set #7, Line 709:</a> <code style="font-family:monospace,monospace"> LOGGER.debug("Packet interpreted")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What's the reason for this debug statement? Seems like this has the potential to spam the debug log. I'd suggest removing it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/e0cbf02c_d283f78a">Patch Set #7, Line 712:</a> <code style="font-family:monospace,monospace"> #interpreted_packet = self._packet_factories.SIPPacketFactory.interpret_packet(packet)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove this line</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/c0c76248_8ee4bf0f">Patch Set #7, Line 753:</a> <code style="font-family:monospace,monospace"> LOGGER.debug("Interpreted packet is empty")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same for this one? How needed is it?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/phones.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/244d122c_56f252da">Patch Set #7, Line 13:</a> <code style="font-family:monospace,monospace">import pjsua2 as pj</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This changes the dependency. The install_prereq script should also be updated.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/9ae81ef9_be3acb4b">Patch Set #7, Line 13:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">import pjsua2 as pj<br>from asterisk import pjsua_mod<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I feel like most, if not all the changes in this file are in response to the upgrade to pjsua2. Is upgrading to pjsua2 a requirement for, or related to python3? If not I'd think these changes should be separated out and done in another commit.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/2ea2317d_b865a48e">Patch Set #7, Line 55:</a> <code style="font-family:monospace,monospace"> LOGGER.info("Phone Account success")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think this would be more helpful if it was to within below the "if (self.num_accts_created != self.num_accts)" block and the number of accounts created was added.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also would it make more sense for it to be a debug message?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/aa38fc39_5bbecb21">Patch Set #7, Line 65:</a> <code style="font-family:monospace,monospace"> LOGGER.info("Creating Phones")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">should this be a debug vs info?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/aeca0255_d3a44127">Patch Set #7, Line 394:</a> <code style="font-family:monospace,monospace"> LOGGER.info("phone attempting to make call")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">seems more like a debug message?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/pjsua_mod.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/f61abf8f_b6f1ea48">Patch Set #7, Line 18:</a> <code style="font-family:monospace,monospace">import pjsua2 as pj</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same question as before. Are the changes here due to moving to python3 or pjsua2? If the latter then separate the changes into another pjsua2 only patch.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/be617531_5d3308d9">Patch Set #7, Line 143:</a> <code style="font-family:monospace,monospace"> time.sleep(1)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In a twisted environment the use of a "sleep" is usually undesired. This will pause/halt all twisted processing, which could cause other test timing issues.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If there is a better way to achieve what you want here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/e73f83df_d9412231">Patch Set #7, Line 184:</a> <code style="font-family:monospace,monospace"> LOGGER.info("ami connect finished!")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is there a way to also print which AMI connection finished? Could be multiple.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/d5fecaeb_d85ef63c">Patch Set #7, Line 260:</a> <code style="font-family:monospace,monospace"> LOGGER.info("worker thread started..")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Seems more of a debug message to me.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/c493f4ff_ba5afa23">Patch Set #7, Line 267:</a> <code style="font-family:monospace,monospace"> LOGGER.info('worker thread exited..')</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Seems more of a debug message to me.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/2e77e49f_bcbd43ea">Patch Set #7, Line 270:</a> <code style="font-family:monospace,monospace"> LOGGER.info("Starting worker thread")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Seems more of a debug message to me.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/656cf103_2ba0d09f">Patch Set #7, Line 345:</a> <code style="font-family:monospace,monospace"> LOGGER.info("Account success")</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Which account?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/pluggable_modules.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/5f17c82f_0ff70ab0">Patch Set #7, Line 53:</a> <code style="font-family:monospace,monospace"> 'nowait': 'False',</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think we can leave the string values alone and just change the field param below. That way configurations don't need to change at all for this.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/cf1c80fb_e909d081">Patch Set #7, Line 127:</a> <code style="font-family:monospace,monospace"> nowait=self.config['nowait'])</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Leave the string 'async' here.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/0255db69_5133ab2f">Patch Set #7, Line 135:</a> <code style="font-family:monospace,monospace"> nowait=self.config['nowait'])</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Leave the string 'async' here.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/realtime_test_module.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/aff0f80e_f239c029">Patch Set #7, Line 12:</a> <code style="font-family:monospace,monospace">import html</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think this module is backwards compat with 2.7?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/9123b3bf_24c13b15">Patch Set #7, Line 14:</a> <code style="font-family:monospace,monospace">from wsgiref.util import request_uri</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This appears unused, so can be removed.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/bbfb052a_777f4ca9">Patch Set #7, Line 284:</a> <code style="font-family:monospace,monospace"> string = '&'.join(['{0}={1}'.format(html.escape(key), html.escape(val))</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The html.escape function appears to have been added in version 3.2, so I don't think is backwards compat.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/test_case.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/9fed91f3_83690476">Patch Set #7, Line 863:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> if 'nowait' not in call_details:<br> call_details['nowait'] = False<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Leave these as 'async'</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/97d148e9_a4728146">Patch Set #7, Line 876:</a> <code style="font-family:monospace,monospace"> nowait=call_details['nowait'],</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Leave the string value as 'async'</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/72f32789_5e9107d2">Patch Set #7, Line 890:</a> <code style="font-family:monospace,monospace"> nowait=call_details['nowait'],</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same here. Leave string as 'async'</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/asterisk/test_runner.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/d3b91fde_449c4ad5">Patch Set #7, Line 18:</a> <code style="font-family:monospace,monospace">from importlib.machinery import SourceFileLoader</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is causing the CI to fail.</p><p style="white-space: pre-wrap; word-wrap: break-word;">This wasn't introduced until 3.1, and so is no in 2.7. Probably need to wrap in a try/except similar to how the yaml import below is handled, or by some other version distinguishing mechanism.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/protocols/ipstack.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/48f43b63_2026e8a9">Patch Set #7, Line 9:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#from construct import Struct, HexDump, Switch, Pass, Computed, Hex, Bytes, setGlobalPrintFullStrings<br>#from construct import Struct, HexDump, Switch, Pass, Computed, Hex, Bytes, setglobalfullprinting<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove commented code</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/ef41d25a_dfdf858f">Patch Set #7, Line 18:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">#setglobalfullprinting(True)<br>#setGlobalPrintFullStrings(True)<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove commented code.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Why is setglobalfullprinting being removed?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File lib/python/protocols/layer4/tcp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/bcd0f3ec_3c842c37">Patch Set #7, Line 9:</a> <code style="font-family:monospace,monospace">setGlobalPrintFullStrings(True)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why is this being removed?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/SIP/sip_blind_transfer/callee_with_reinvite/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/024792c0_2373ab1e">Patch Set #7, Line 10:</a> <code style="font-family:monospace,monospace">from builtins import bytes</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">builtins 2.7 compat? I think it'll require an install of "futures".</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/SIP/sip_blind_transfer/caller_refer_only/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/8d9a7a77_52a5c48e">Patch Set #7, Line 10:</a> <code style="font-family:monospace,monospace">from builtins import bytes</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think this is in 2.7</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/SIP/sip_blind_transfer/caller_with_reinvite/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/11711de9_c549a03d">Patch Set #7, Line 10:</a> <code style="font-family:monospace,monospace">from builtins import bytes</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Here too</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/pjsip/rtp/bind_rtp_to_media_address/rtp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/a51e9c63_94949e5f">Patch Set #7, Line 21:</a> <code style="font-family:monospace,monospace">def datagramReceived(self, data, addr):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Hrm, this appears to be a twisted library version difference. I don't believe this will be backwards compatible with whatever twisted python2.7 version is available?</p><p style="white-space: pre-wrap; word-wrap: break-word;">You might be able to "monkey patch" this somehow based on twisted method(s) available.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/pjsip/rtp/rtp_keepalive/base/rtp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/eae50cf8_917c7b8e">Patch Set #7, Line 28:</a> <code style="font-family:monospace,monospace"> def datagramReceived(self, data, addr):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same problem here. This is a twisted version incompatibility issue. You'll need to figure out a way to make such things backwards compat.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/pjsip/rtp/rtp_keepalive/direct_media/rtp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/e2c22cc0_9250fbc1">Patch Set #7, Line 19:</a> <code style="font-family:monospace,monospace"> def datagramReceived(self, data, addr):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This needs to be backwards compat</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/pjsip/subscriptions/rls/rls_test.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/05274117_a49f64b2">Patch Set #7, Line 33:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> LOGGER.debug("request line")<br> LOGGER.debug(packet.request_line)<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The addition of debug/info logging should be in a separate patch.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/channels/pjsip/transfers/attended_transfer/nominal/callee_local_app/test-config.yaml:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/b7977e0b_535c81af">Patch Set #7, Line 178:</a> <code style="font-family:monospace,monospace"> #- python : pjsua2</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why commented out?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/codecs/audio_analyzer.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/e6cb3737_9d402c26">Patch Set #7, Line 188:</a> <code style="font-family:monospace,monospace"> 'nowait': 'True'</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This can be left as 'async' if other references change</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/hep/hep_capture_node.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/7947dd85_4e88c6ee">Patch Set #7, Line 85:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"> self.src_addr = socket.inet_ntop(socket.AF_INET, bytes(src_addr.ipv4_addr,"utf-8"))<br> self.dst_addr = socket.inet_ntop(socket.AF_INET, bytes(dst_addr.ipv4_addr,"utf-8"))<br> elif self.ip_family == IP_FAMILY.v6:<br> self.src_addr = socket.inet_ntop(socket.AF_INET6, bytes(src_addr.ipv6_addr,"utf-8"))<br> self.dst_addr = socket.inet_ntop(socket.AF_INET6, bytes(dst_addr.ipv6_addr,"utf-8"))<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">"bytes" is not backwards compat</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/e6be7112_05bb8826">Patch Set #7, Line 147:</a> <code style="font-family:monospace,monospace"> def datagramReceived(self, data, addr):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Twisted compat issue here too</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/rest_api/applications/stasisstatus/test_case.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/d5872f46_f40c3590">Patch Set #7, Line 10:</a> <code style="font-family:monospace,monospace">from builtins import StopIteration, super</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What are these imports for?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/rest_api/channels/redirect/nominal/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/0ad2e21a_d8654ca4">Patch Set #7, Line 12:</a> <code style="font-family:monospace,monospace">from builtins import range, super</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is this line needed? If so is "super" strictly required?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/rest_api/chunked-transfer/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/e24bb7c0_cb720787">Patch Set #7, Line 10:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">from asyncio.log import logger<br>from builtins import bytes, len, list, str<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm not sure all of this is 2.7 backwards compat. For sure the asyncio stuff.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/d324afb2_7e08f7a9">Patch Set #7, Line 33:</a> <code style="font-family:monospace,monospace">bytes</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think this function call is backwards compat</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/c3ac85e3_955d9c40">Patch Set #7, Line 34:</a> <code style="font-family:monospace,monospace"># return url</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">remove</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/445b28a3_9a6c71c6">Patch Set #7, Line 67:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"># data=bytes(chunkizer(b"{ b'variable': b'foo', b'value': b'bar'' }"),"utf-8"),<br># data={b'variable': b'foo', b'value': b'bar'},<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove unused code</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/604b2029_411804ac">Patch Set #7, Line 70:</a> <code style="font-family:monospace,monospace"> auth = (USERNAME, PASSWORD))</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is this a python3 compat change? If so is it backward compat with the library?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/rest_api/request-bodies/run-test:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/4924ec37_ef4f8c3b">Patch Set #7, Line 28:</a> <code style="font-family:monospace,monospace"> params = urllib.parse.urlencode({'api_key': 'testsuite:testsuite'})</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think this too is 2.7 compat</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/rtp/strict_rtp/strict_rtp_seqno/strict_rtp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/dca4ef89_0254b7a1">Patch Set #7, Line 35:</a> <code style="font-family:monospace,monospace"> def datagramReceived(self, data, addr):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Another twisted compat issue here</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/rtp/strict_rtp/strict_rtp_yes/strict_rtp.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/testsuite/+/18199/comment/fa32df3c_58de8245">Patch Set #7, Line 37:</a> <code style="font-family:monospace,monospace"> def datagramReceived(self, data, addr):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Twisted compat issue</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/testsuite/+/18199">change 18199</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/testsuite/+/18199"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: testsuite </div>
<div style="display:none"> Gerrit-Branch: development/16/python3 </div>
<div style="display:none"> Gerrit-Change-Id: Ic7a1d72b174df59107370fcb03fae9dc4cdfc9d3 </div>
<div style="display:none"> Gerrit-Change-Number: 18199 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Attention: Michael Bradeen <mbradeen@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 25 Apr 2022 23:30:53 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>