[Asterisk-code-review] testsuite: Python 3 compatibility inital commit (testsuite[development/16/python3])

Tue May 10 10:11:55 CDT 2022

Attention is currently required from: Joshua Colp, Kevin Harwell.
Michael Bradeen has posted comments on this change. ( https://gerrit.asterisk.org/c/testsuite/+/18199 )

Change subject: testsuite: Python 3 compatibility inital commit
......................................................................

Patch Set 8:

(19 comments)

File lib/python/asterisk/astdicts.py:

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/46d6b4e3_c02df040 
PS7, Line 274: #            print("__setitem__ key = ", key, " val = ", val)
> Since this began as commented code I'd just remove it vs updating it.
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/c1e21a2f_b0c1712f 
PS7, Line 278: #        print("inserting key = ", key, " val = ", val)
> Same here. This line can be removed altogether.
Done

File lib/python/asterisk/pcap.py:

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/9a7275b0_e53513b2 
PS7, Line 709:                 LOGGER.debug("Packet interpreted")
> What's the reason for this debug statement? Seems like this has the potential to spam the debug log. […]
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/eecdf561_5c6138a1 
PS7, Line 712:         #interpreted_packet = self._packet_factories.SIPPacketFactory.interpret_packet(packet)
> Remove this line
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/9253983e_eb419509 
PS7, Line 753:             LOGGER.debug("Interpreted packet is empty")
> Same for this one? How needed is it?
Done

File lib/python/asterisk/phones.py:

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/7b258773_81b21631 
PS7, Line 13: import pjsua2 as pj
> This changes the dependency. The install_prereq script should also be updated.
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/d758c54f_66f59b65 
PS7, Line 13: import pjsua2 as pj
            : from asterisk import pjsua_mod
> I feel like most, if not all the changes in this file are in response to the upgrade to pjsua2. […]
unfortunately, pjsua '1' libs do not work with python3 so this is a requirement.

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/1ea7c224_0eaeb7ee 
PS7, Line 55:         LOGGER.info("Phone Account success")
> I think this would be more helpful if it was to within below the "if (self. […]
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/f3e0fd41_216bcdcd 
PS7, Line 65:         LOGGER.info("Creating Phones")
> should this be a debug vs info?
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/02820150_cc09104f 
PS7, Line 394:     LOGGER.info("phone attempting to make call")
> seems more like a debug message?
Done

File lib/python/asterisk/pjsua_mod.py:

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/28d049de_4ad29cc6 
PS7, Line 143:         time.sleep(1)
> In a twisted environment the use of a "sleep" is usually undesired. […]
doesn't need to sleep at all actually.

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/39d58863_02c528e3 
PS7, Line 184:         LOGGER.info("ami connect finished!")
> Is there a way to also print which AMI connection finished? Could be multiple.
made it a debug, the log wrapper give the rest of the info, but this is only actually useful for debugging pjsua2 thread issues.

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/2b26031b_24b5fc7a 
PS7, Line 260:         LOGGER.info("worker thread started..")
> Seems more of a debug message to me.
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/23af4795_97c4beb7 
PS7, Line 267:         LOGGER.info('worker thread exited..')
> Seems more of a debug message to me.
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/c7c9b992_950c0b3b 
PS7, Line 270:         LOGGER.info("Starting worker thread")
> Seems more of a debug message to me.
Done

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/72c63ea4_bec5969a 
PS7, Line 345:         LOGGER.info("Account success")
> Which account?
Done

File lib/python/asterisk/realtime_test_module.py:

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/7c60d43c_bf6d70cd 
PS7, Line 12: import html
> I don't think this module is backwards compat with 2. […]
Doing in one go, so no backwards compat.

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/f010ed4a_4da797b1 
PS7, Line 284:         string = '&'.join(['{0}={1}'.format(html.escape(key), html.escape(val))
> The html.escape function appears to have been added in version 3. […]
Doing in one go, so no backwards compat.

File tests/channels/pjsip/transfers/attended_transfer/nominal/callee_local_app/test-config.yaml:

https://gerrit.asterisk.org/c/testsuite/+/18199/comment/5f03b610_daea9fa2 
PS7, Line 178:         #- python : pjsua2
> Why commented out?
Done

-- 
To view, visit https://gerrit.asterisk.org/c/testsuite/+/18199
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: testsuite
Gerrit-Branch: development/16/python3
Gerrit-Change-Id: Ic7a1d72b174df59107370fcb03fae9dc4cdfc9d3
Gerrit-Change-Number: 18199
Gerrit-PatchSet: 8
Gerrit-Owner: Michael Bradeen <mbradeen at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Tue, 10 May 2022 15:11:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220510/265e1640/attachment-0001.html>