Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: 2.2
Choose a base branch
from

Conversation

KCarmichael
Copy link

@KCarmichael KCarmichael commented Jan 27, 2024

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:

  • Migrated to telnetlib3 for improved handling of Telnet commands and options.
  • Upgraded TelnetConnection with error handling in send and close methods.
  • Refined AsyncioTelnetServer by inheriting from telnetlib3.TelnetServer, featuring streamlined command processing and enhanced logging.
  • Removed unnecessary dependencies and imports, simplifying the codebase.
  • Retained the core execution flow, adapting it to the new class structures.

Observation for Future Improvement:

  • Some NVT commands are still marked as unhandled in debug logs despite having the necessary handling statements. This behaviour may be related to certain user-reported bugs.
  • Further comprehensive testing and diagnostics are needed to understand and resolve these issues.

Next Steps:

  • A detailed code review focusing on prerequisites and requirements is suggested. This will determine which functions can be effectively delegated to telnetlib3 without negatively impacting other GNS3 modules.
  • The goal is to ensure optimal integration of telnetlib3, maximizing efficiency while preserving the integrity and functionality of the broader GNS3 ecosystem.

…3 library, enhanced command handling and error management..

Extended description:
- Migrated to telnetlib3 for improved handling of Telnet commands and options.
- Upgraded TelnetConnection with error handling in send and close methods.
- Refined AsyncioTelnetServer by inheriting from telnetlib3.TelnetServer, featuring streamlined command processing and enhanced logging.
- Removed unnecessary dependencies and imports, simplifying the codebase.
- Retained the core execution flow, adapting it to the new class structures.

Observation for Future Improvement:
- Some NVT commands are still marked as unhandled in debug logs despite having the necessary handling statments. This behaviour may be related to certain user-reported bugs.
- Further comprehensive testing and diagnostics are needed to understand and resolve these issues.

Next Steps:
- A detailed code review focusing on prerequisites and requirements is suggested. This will determine which functions can be effectively delegated to telnetlib3 without negatively impacting other GNS3 modules.
- The goal is to ensure optimal integration of telnetlib3, maximizing efficiency while preserving the integrity and functionality of the broader GNS3 ecosystem.
@KCarmichael
Copy link
Author

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

@KCarmichael
Copy link
Author

@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
Copy link
Member

grossmj commented Feb 7, 2024

@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 grossmj changed the base branch from master to 2.2 February 9, 2024 06:01
@grossmj
Copy link
Member

grossmj commented Feb 9, 2024

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

@KCarmichael
Copy link
Author

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

@grossmj
Copy link
Member

grossmj commented Feb 10, 2024

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 👍

@grossmj grossmj deleted the branch GNS3:2.2 July 8, 2024 22:30
@grossmj grossmj closed this Jul 8, 2024
@cristian-ciobanu
Copy link

@grossmj is this pull request merged into the v3 branch for GNS3 or it was closed without merging and v3 is still running on the old telnet server implementation which also has that console freeze issue because I see the issue is still open #2289?

@grossmj
Copy link
Member

grossmj commented Nov 20, 2024

@grossmj is this pull request merged into the v3 branch for GNS3 or it was closed without merging and v3 is still running on the old telnet server implementation which also has that console freeze issue because I see the issue is still open #2289?

Correct, this PR was closed (automatically?) without merging. There are still some pending issues: #2289 (comment)

I think the best is to have 3.0 out and stable before focusing on fixing the PR and merging in 3.1

@grossmj grossmj reopened this Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants