Skip to content

Add missing LinuxSer2Net class#1

Open
jrrech wants to merge 5 commits intoevedovelli:mainfrom
jrrech:main
Open

Add missing LinuxSer2Net class#1
jrrech wants to merge 5 commits intoevedovelli:mainfrom
jrrech:main

Conversation

@jrrech
Copy link

@jrrech jrrech commented Oct 18, 2021

Also fix typos on connections/Ser2Net.py.

ser2net,
username=username,
password=password,
pty_winsize_cols=SSH_PTY_WINSIZE_COLS,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you should import and use the PTY_WINSIZE_COLS from Ser2Net instead

####################################################################################################
## Ser2NetLinux

class Ser2NetLinux(SshLinux):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it extendind the SshLinux instead of Linux? Is it only for the implementation of the logout function? If so, move it to the Linux class and extend the Ser2NetLinux from Linux, because the SshLinux may have invalid methods for Ser2NetLinux in the future.

Copy link
Author

@jrrech jrrech Oct 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I tried to reuse the login function as well, but it needed some tuning for Ser2Net in the end. It does make more sense extending Linux, though.


class Ser2NetLinux(SshLinux):
""" Connects to a remote Linux Shell using SSH.
Core implementation is done by Ssh and Linux.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace SSH with Ser2Net in the comments above

Also fix typos on connections/Ser2Net.py.
Copy link
Owner

@evedovelli evedovelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

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.

2 participants

Comments