-
Notifications
You must be signed in to change notification settings - Fork 43
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
Add support for multiple ssh backends #282
Conversation
Functional tests on
|
Updated functional test ids for parametrized scenario tests, to show the scenario file name instead of the default, less informative, ids created by pytest. Before:
After:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work overall! Just some relatively minor notes.
broker/session.py
Outdated
if SSH_BACKEND == "ansible-pylibssh": | ||
from broker.ssh_session_pylibssh import InteractiveShell, Session | ||
elif SSH_BACKEND == "hussh": | ||
from broker.ssh_session_hussh import Session | ||
elif SSH_BACKEND in ("ssh2-python", "ssh2-python312"): | ||
from broker.ssh_session_ssh2 import InteractiveShell, Session # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more of an organization suggestion, but I think these shouldn't live at this level. I think it makes sense to move these under the binds directory and strip ssh_session
from the module name.
if SSH_BACKEND == "ansible-pylibssh": | |
from broker.ssh_session_pylibssh import InteractiveShell, Session | |
elif SSH_BACKEND == "hussh": | |
from broker.ssh_session_hussh import Session | |
elif SSH_BACKEND in ("ssh2-python", "ssh2-python312"): | |
from broker.ssh_session_ssh2 import InteractiveShell, Session # noqa: F401 | |
if SSH_BACKEND == "ansible-pylibssh": | |
from broker.binds.pylibssh import InteractiveShell, Session | |
elif SSH_BACKEND == "hussh": | |
from broker.binds.hussh import Session | |
elif SSH_BACKEND in ("ssh2-python", "ssh2-python312"): | |
from broker.binds.ssh2 import InteractiveShell, Session # noqa: F401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved modules to binds directory.
broker/session.py
Outdated
@contextmanager | ||
def tail_file(self, filename): | ||
"""Simulate tailing a file on the remote host. | ||
from broker.ssh_session import BaseSession as Session |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't the ssh_session
module's contents live in this module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had originally thought of using BaseSession
as a base class for the individual backend classes. I've updated the import logic to create the default session class if a backend can't be imported.
919c03b
to
7f317dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK, thanks for taking this on!
ssh_backend: ssh2-python312
default setting tobroker_settings.yaml.example
. To usehussh
oransible-pylibssh
, install the package and then override the setting in the localbroker_settings.yaml
file, or with theBROKER_SSH_BACKEND
environment variable:Verified functional tests: