On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.lin...@pantheon.tech> wrote:
> Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech> > --- > .../interactive_remote_session.py | 36 +++---- > .../remote_session/interactive_shell.py | 99 +++++++++++-------- > dts/framework/remote_session/python_shell.py | 26 ++++- > dts/framework/remote_session/testpmd_shell.py | 58 +++++++++-- > 4 files changed, 149 insertions(+), 70 deletions(-) > > diff --git a/dts/framework/remote_session/interactive_remote_session.py > b/dts/framework/remote_session/interactive_remote_session.py > index 098ded1bb0..1cc82e3377 100644 > --- a/dts/framework/remote_session/interactive_remote_session.py > +++ b/dts/framework/remote_session/interactive_remote_session.py > @@ -22,27 +22,23 @@ > class InteractiveRemoteSession: > """SSH connection dedicated to interactive applications. > > - This connection is created using paramiko and is a persistent > connection to the > - host. This class defines methods for connecting to the node and > configures this > - connection to send "keep alive" packets every 30 seconds. Because > paramiko attempts > - to use SSH keys to establish a connection first, providing a password > is optional. > - This session is utilized by InteractiveShells and cannot be > interacted with > - directly. > - > - Arguments: > - node_config: Configuration class for the node you are connecting > to. > - _logger: Desired logger for this session to use. > + The connection is created using `paramiko < > https://docs.paramiko.org/en/latest/>`_ > + and is a persistent connection to the host. This class defines the > methods for connecting > + to the node and configures the connection to send "keep alive" > packets every 30 seconds. > + Because paramiko attempts to use SSH keys to establish a connection > first, providing > + a password is optional. This session is utilized by InteractiveShells > + and cannot be interacted with directly. > > Attributes: > - hostname: Hostname that will be used to initialize a connection > to the node. > - ip: A subsection of hostname that removes the port for the > connection if there > + hostname: The hostname that will be used to initialize a > connection to the node. > + ip: A subsection of `hostname` that removes the port for the > connection if there > is one. If there is no port, this will be the same as > hostname. > - port: Port to use for the ssh connection. This will be extracted > from the > - hostname if there is a port included, otherwise it will > default to 22. > + port: Port to use for the ssh connection. This will be extracted > from `hostname` > + if there is a port included, otherwise it will default to > ``22``. > username: User to connect to the node with. > password: Password of the user connecting to the host. This will > default to an > empty string if a password is not provided. > - session: Underlying paramiko connection. > + session: The underlying paramiko connection. > > Raises: > SSHConnectionError: There is an error creating the SSH connection. > @@ -58,9 +54,15 @@ class InteractiveRemoteSession: > _node_config: NodeConfiguration > _transport: Transport | None > > - def __init__(self, node_config: NodeConfiguration, _logger: DTSLOG) > -> None: > + def __init__(self, node_config: NodeConfiguration, logger: DTSLOG) -> > None: > + """Connect to the node during initialization. > + > + Args: > + node_config: The test run configuration of the node to > connect to. > + logger: The logger instance this session will use. > + """ > self._node_config = node_config > - self._logger = _logger > + self._logger = logger > self.hostname = node_config.hostname > self.username = node_config.user > self.password = node_config.password if node_config.password else > "" > diff --git a/dts/framework/remote_session/interactive_shell.py > b/dts/framework/remote_session/interactive_shell.py > index 4db19fb9b3..b158f963b6 100644 > --- a/dts/framework/remote_session/interactive_shell.py > +++ b/dts/framework/remote_session/interactive_shell.py > @@ -3,18 +3,20 @@ > > """Common functionality for interactive shell handling. > > -This base class, InteractiveShell, is meant to be extended by other > classes that > -contain functionality specific to that shell type. These derived classes > will often > -modify things like the prompt to expect or the arguments to pass into the > application, > -but still utilize the same method for sending a command and collecting > output. How > -this output is handled however is often application specific. If an > application needs > -elevated privileges to start it is expected that the method for gaining > those > -privileges is provided when initializing the class. > +The base class, :class:`InteractiveShell`, is meant to be extended by > subclasses that contain > +functionality specific to that shell type. These subclasses will often > modify things like > +the prompt to expect or the arguments to pass into the application, but > still utilize > +the same method for sending a command and collecting output. How this > output is handled however > +is often application specific. If an application needs elevated > privileges to start it is expected > +that the method for gaining those privileges is provided when > initializing the class. > + > +The :option:`--timeout` command line argument and the > :envvar:`DTS_TIMEOUT` > +environment variable configure the timeout of getting the output from > command execution. > """ > > from abc import ABC > from pathlib import PurePath > -from typing import Callable > +from typing import Callable, ClassVar > > from paramiko import Channel, SSHClient, channel # type: ignore[import] > > @@ -30,28 +32,6 @@ class InteractiveShell(ABC): > and collecting input until reaching a certain prompt. All interactive > applications > will use the same SSH connection, but each will create their own > channel on that > session. > - > - Arguments: > - interactive_session: The SSH session dedicated to interactive > shells. > - logger: Logger used for displaying information in the console. > - get_privileged_command: Method for modifying a command to allow > it to use > - elevated privileges. If this is None, the application will > not be started > - with elevated privileges. > - app_args: Command line arguments to be passed to the application > on startup. > - timeout: Timeout used for the SSH channel that is dedicated to > this interactive > - shell. This timeout is for collecting output, so if reading > from the buffer > - and no output is gathered within the timeout, an exception is > thrown. > - > - Attributes > - _default_prompt: Prompt to expect at the end of output when > sending a command. > - This is often overridden by derived classes. > - _command_extra_chars: Extra characters to add to the end of every > command > - before sending them. This is often overridden by derived > classes and is > - most commonly an additional newline character. > - path: Path to the executable to start the interactive application. > - dpdk_app: Whether this application is a DPDK app. If it is, the > build > - directory for DPDK on the node will be prepended to the path > to the > - executable. > """ > > _interactive_session: SSHClient > @@ -61,10 +41,22 @@ class InteractiveShell(ABC): > _logger: DTSLOG > _timeout: float > _app_args: str > - _default_prompt: str = "" > - _command_extra_chars: str = "" > - path: PurePath > - dpdk_app: bool = False > + > + #: Prompt to expect at the end of output when sending a command. > + #: This is often overridden by subclasses. > + _default_prompt: ClassVar[str] = "" > + > + #: Extra characters to add to the end of every command > + #: before sending them. This is often overridden by subclasses and is > + #: most commonly an additional newline character. > + _command_extra_chars: ClassVar[str] = "" > + > + #: Path to the executable to start the interactive application. > + path: ClassVar[PurePath] > + > + #: Whether this application is a DPDK app. If it is, the build > directory > + #: for DPDK on the node will be prepended to the path to the > executable. > + dpdk_app: ClassVar[bool] = False > > def __init__( > self, > @@ -74,6 +66,19 @@ def __init__( > app_args: str = "", > timeout: float = SETTINGS.timeout, > ) -> None: > + """Create an SSH channel during initialization. > + > + Args: > + interactive_session: The SSH session dedicated to interactive > shells. > + logger: The logger instance this session will use. > + get_privileged_command: A method for modifying a command to > allow it to use > + elevated privileges. If :data:`None`, the application > will not be started > + with elevated privileges. > + app_args: The command line arguments to be passed to the > application on startup. > + timeout: The timeout used for the SSH channel that is > dedicated to this interactive > + shell. This timeout is for collecting output, so if > reading from the buffer > + and no output is gathered within the timeout, an > exception is thrown. > + """ > self._interactive_session = interactive_session > self._ssh_channel = self._interactive_session.invoke_shell() > self._stdin = self._ssh_channel.makefile_stdin("w") > @@ -90,6 +95,10 @@ def _start_application(self, get_privileged_command: > Callable[[str], str] | None > > This method is often overridden by subclasses as their process for > starting may look different. > + > + Args: > + get_privileged_command: A function (but could be any > callable) that produces > + the version of the command with elevated privileges. > """ > start_command = f"{self.path} {self._app_args}" > if get_privileged_command is not None: > @@ -97,16 +106,24 @@ def _start_application(self, get_privileged_command: > Callable[[str], str] | None > self.send_command(start_command) > > def send_command(self, command: str, prompt: str | None = None) -> > str: > - """Send a command and get all output before the expected ending > string. > + """Send `command` and get all output before the expected ending > string. > > Lines that expect input are not included in the stdout buffer, so > they cannot > - be used for expect. For example, if you were prompted to log into > something > - with a username and password, you cannot expect "username:" > because it won't > - yet be in the stdout buffer. A workaround for this could be > consuming an > - extra newline character to force the current prompt into the > stdout buffer. > + be used for expect. > + > + Example: > + If you were prompted to log into something with a username > and password, > + you cannot expect ``username:`` because it won't yet be in > the stdout buffer. > + A workaround for this could be consuming an extra newline > character to force > + the current `prompt` into the stdout buffer. > + > + Args: > + command: The command to send. > + prompt: After sending the command, `send_command` will be > expecting this string. > + If :data:`None`, will use the class's default prompt. > > Returns: > - All output in the buffer before expected string > + All output in the buffer before expected string. > """ > self._logger.info(f"Sending: '{command}'") > if prompt is None: > @@ -124,8 +141,10 @@ def send_command(self, command: str, prompt: str | > None = None) -> str: > return out > > def close(self) -> None: > + """Properly free all resources.""" > self._stdin.close() > self._ssh_channel.close() > > def __del__(self) -> None: > + """Make sure the session is properly closed before deleting the > object.""" > self.close() > diff --git a/dts/framework/remote_session/python_shell.py > b/dts/framework/remote_session/python_shell.py > index cc3ad48a68..ccfd3783e8 100644 > --- a/dts/framework/remote_session/python_shell.py > +++ b/dts/framework/remote_session/python_shell.py > @@ -1,12 +1,32 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > +"""Python interactive shell. > + > +Typical usage example in a TestSuite:: > + > + from framework.remote_session import PythonShell > + python_shell = self.tg_node.create_interactive_shell( > + PythonShell, timeout=5, privileged=True > + ) > + python_shell.send_command("print('Hello World')") > + python_shell.close() > +""" > + > from pathlib import PurePath > +from typing import ClassVar > > from .interactive_shell import InteractiveShell > > > class PythonShell(InteractiveShell): > - _default_prompt: str = ">>>" > - _command_extra_chars: str = "\n" > - path: PurePath = PurePath("python3") > + """Python interactive shell.""" > + > + #: Python's prompt. > + _default_prompt: ClassVar[str] = ">>>" > + > + #: This forces the prompt to appear after sending a command. > + _command_extra_chars: ClassVar[str] = "\n" > + > + #: The Python executable. > + path: ClassVar[PurePath] = PurePath("python3") > diff --git a/dts/framework/remote_session/testpmd_shell.py > b/dts/framework/remote_session/testpmd_shell.py > index 08ac311016..79481e845c 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -1,41 +1,79 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2023 University of New Hampshire > > Should you add to the copyright here for adding comments? > +"""Testpmd interactive shell. > + > +Typical usage example in a TestSuite:: > + > + testpmd_shell = self.sut_node.create_interactive_shell( > + TestPmdShell, privileged=True > + ) > + devices = testpmd_shell.get_devices() > + for device in devices: > + print(device) > + testpmd_shell.close() > +""" > + > from pathlib import PurePath > -from typing import Callable > +from typing import Callable, ClassVar > > from .interactive_shell import InteractiveShell > > > class TestPmdDevice(object): > + """The data of a device that testpmd can recognize. > + > + Attributes: > + pci_address: The PCI address of the device. > + """ > + > pci_address: str > > def __init__(self, pci_address_line: str): > + """Initialize the device from the testpmd output line string. > + > + Args: > + pci_address_line: A line of testpmd output that contains a > device. > + """ > self.pci_address = pci_address_line.strip().split(": ")[1].strip() > > def __str__(self) -> str: > + """The PCI address captures what the device is.""" > return self.pci_address > > > class TestPmdShell(InteractiveShell): > - path: PurePath = PurePath("app", "dpdk-testpmd") > - dpdk_app: bool = True > - _default_prompt: str = "testpmd>" > - _command_extra_chars: str = "\n" # We want to append an extra > newline to every command > + """Testpmd interactive shell. > + > + The testpmd shell users should never use > + the :meth:`~.interactive_shell.InteractiveShell.send_command` method > directly, but rather > + call specialized methods. If there isn't one that satisfies a need, > it should be added. > + """ > + > + #: The path to the testpmd executable. > + path: ClassVar[PurePath] = PurePath("app", "dpdk-testpmd") > + > + #: Flag this as a DPDK app so that it's clear this is not a system > app and > + #: needs to be looked in a specific path. > + dpdk_app: ClassVar[bool] = True > + > + #: The testpmd's prompt. > + _default_prompt: ClassVar[str] = "testpmd>" > + > + #: This forces the prompt to appear after sending a command. > + _command_extra_chars: ClassVar[str] = "\n" > > def _start_application(self, get_privileged_command: Callable[[str], > str] | None) -> None: > - """See "_start_application" in InteractiveShell.""" > self._app_args += " -- -i" > super()._start_application(get_privileged_command) > > def get_devices(self) -> list[TestPmdDevice]: > - """Get a list of device names that are known to testpmd > + """Get a list of device names that are known to testpmd. > > - Uses the device info listed in testpmd and then parses the output > to > - return only the names of the devices. > + Uses the device info listed in testpmd and then parses the output. > > Returns: > - A list of strings representing device names (e.g. > 0000:14:00.1) > + A list of devices. > """ > dev_info: str = self.send_command("show device info all") > dev_list: list[TestPmdDevice] = [] > -- > 2.34.1 > >
