GNS3 / gns3-server

GNS3 server
GNU General Public License v3.0
768 stars 258 forks source link

Summary: Refactored ..utils.asyncio.telnet_server.py to use telnetlib3… #2342

Open KCarmichael opened 5 months ago

KCarmichael commented 5 months ago

Related to Issue: #2289 - Re-write Telnet server using telnetlib3 link

Summary: Refactored ..utils.asyncio.telnet_server.py to use telnetlib3 library, enhanced command handling and error management.

Extended description:

Observation for Future Improvement:

Next Steps:

KCarmichael commented 5 months ago

@grossmj - would you be able to re-trigger the PR tests?, Now the requirements file is updated I believe tests should pass.

KCarmichael commented 5 months ago

@grossmj - it would seem telnetlib3 is not compatible with python 3.6, it may have originally but I believe they dropped 3.6 support in favour of focusing on 3.9>

That being said, 3.7 should be compatible.

What do we want to do regarding telnetlib3 integration?

grossmj commented 4 months ago

@grossmj - it would seem telnetlib3 is not compatible with python 3.6, it may have originally but I believe they dropped 3.6 support in favour of focusing on 3.9>

I remember the main reason we keep Python 3.6 support (at least for GNS3 2.2.x) is because of Ubuntu 18.04 Bionic LTS which come with Python 3.6 by default and it comes end-of-life in April 2028 (https://wiki.ubuntu.com/Releases). That being said, it's becoming a PITA to maintain compatibility with Python 3.6 because so many of our dependencies have already dropped support, I don't think many of our users run GNS3 on Ubuntu 18.04 and we could just say it is "End of Standard Support" (June 2023) already.

What do we want to do regarding telnetlib3 integration?

I think we have 2 choices here, drop Python 3.6 support or add the telnetlib3 integration to GNS3 v3 only which has support for Python 3.8 to 3.12.

grossmj commented 4 months ago

I have decided to drop support for Python 3.6 in v2.2.x.

KCarmichael commented 4 months ago

That makes sense, so what needs to happen on this PR now?

grossmj commented 4 months ago

I want to run some more tests and also look into that 3 special characters issue that had been reported, see if I can replicate it. I will merge once everything is okay. Thanks again 👍