<p> Attention is currently required from: Joshua Colp, Kevin Harwell. </p>
<p><a href="https://gerrit.asterisk.org/c/testsuite/+/18199">View Change</a></p><p>19 comments:</p><ul style="list-style: none; padding: 0;"><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/46d6b4e3_c02df040">Patch Set #7, Line 274:</a> <code style="font-family:monospace,monospace">#            print("__setitem__ key = ", key, " val = ", val)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Since this began as commented code I'd just remove it vs updating it.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/c1e21a2f_b0c1712f">Patch Set #7, Line 278:</a> <code style="font-family:monospace,monospace">#        print("inserting key = ", key, " val = ", val)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same here. This line can be removed altogether.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/9a7275b0_e53513b2">Patch Set #7, Line 709:</a> <code style="font-family:monospace,monospace">                LOGGER.debug("Packet interpreted")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">What's the reason for this debug statement? Seems like this has the potential to spam the debug log. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/eecdf561_5c6138a1">Patch Set #7, Line 712:</a> <code style="font-family:monospace,monospace">        #interpreted_packet = self._packet_factories.SIPPacketFactory.interpret_packet(packet)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Remove this line</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/9253983e_eb419509">Patch Set #7, Line 753:</a> <code style="font-family:monospace,monospace">            LOGGER.debug("Interpreted packet is empty")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same for this one? How needed is it?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/7b258773_81b21631">Patch Set #7, Line 13:</a> <code style="font-family:monospace,monospace">import pjsua2 as pj</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This changes the dependency. The install_prereq script should also be updated.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/d758c54f_66f59b65">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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I feel like most, if not all the changes in this file are in response to the upgrade to pjsua2. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">unfortunately, pjsua '1' libs do not work with python3 so this is a requirement.</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/1ea7c224_0eaeb7ee">Patch Set #7, Line 55:</a> <code style="font-family:monospace,monospace">        LOGGER.info("Phone Account success")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think this would be more helpful if it was to within below the "if (self. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/f3e0fd41_216bcdcd">Patch Set #7, Line 65:</a> <code style="font-family:monospace,monospace">        LOGGER.info("Creating Phones")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">should this be a debug vs info?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/02820150_cc09104f">Patch Set #7, Line 394:</a> <code style="font-family:monospace,monospace">    LOGGER.info("phone attempting to make call")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">seems more like a debug message?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/28d049de_4ad29cc6">Patch Set #7, Line 143:</a> <code style="font-family:monospace,monospace">        time.sleep(1)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">In a twisted environment the use of a "sleep" is usually undesired. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">doesn't need to sleep at all actually.</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/39d58863_02c528e3">Patch Set #7, Line 184:</a> <code style="font-family:monospace,monospace">        LOGGER.info("ami connect finished!")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Is there a way to also print which AMI connection finished? Could be multiple.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">made it a debug, the log wrapper give the rest of the info, but this is only actually useful for debugging pjsua2 thread issues.</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/2b26031b_24b5fc7a">Patch Set #7, Line 260:</a> <code style="font-family:monospace,monospace">        LOGGER.info("worker thread started..")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Seems more of a debug message to me.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/23af4795_97c4beb7">Patch Set #7, Line 267:</a> <code style="font-family:monospace,monospace">        LOGGER.info('worker thread exited..')</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Seems more of a debug message to me.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/c7c9b992_950c0b3b">Patch Set #7, Line 270:</a> <code style="font-family:monospace,monospace">        LOGGER.info("Starting worker thread")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Seems more of a debug message to me.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/72c63ea4_bec5969a">Patch Set #7, Line 345:</a> <code style="font-family:monospace,monospace">        LOGGER.info("Account success")</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Which account?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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/7c60d43c_bf6d70cd">Patch Set #7, Line 12:</a> <code style="font-family:monospace,monospace">import html</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I don't think this module is backwards compat with 2. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Doing in one go, so no 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/f010ed4a_4da797b1">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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The html.escape function appears to have been added in version 3. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Doing in one go, so no backwards compat.</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/5f03b610_daea9fa2">Patch Set #7, Line 178:</a> <code style="font-family:monospace,monospace">        #- python : pjsua2</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why commented out?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</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: 8 </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-CC: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 10 May 2022 15:11:55 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>