From: Nicholas Pratte <npratte@iol.unh.edu>
To: Luca Vizzarro <luca.vizzarro@arm.com>
Cc: dev@dpdk.org, Paul Szczepanek <paul.szczepanek@arm.com>,
Patrick Robb <probb@iol.unh.edu>
Subject: Re: [PATCH v2 4/7] dts: revert back to a single InteractiveShell
Date: Fri, 25 Apr 2025 14:17:56 -0400 [thread overview]
Message-ID: <CAKXZ7eis5F0qQr-xe+S-HjXQFknUXuNuBnTEukDng_6YVAcvaQ@mail.gmail.com> (raw)
In-Reply-To: <20250314131857.1298247-5-luca.vizzarro@arm.com>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
On Fri, Mar 14, 2025 at 9:19 AM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> Previously InteractiveShell was split into two classes to differentiate
> a shell which execution must be controlled in a tight scope through a
> context manager, from a more looser approach. With the addition of the
> shell pool this is no longer needed as the management of shells is now
> delegated to the test run instead of the test suites.
>
> Revert back to a single InteractiveShell to simplify the code. Keep the
> context manager implementation but also render start_application and
> close public methods again.
>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> ---
> dts/framework/remote_session/dpdk_app.py | 4 +-
> dts/framework/remote_session/dpdk_shell.py | 6 +-
> .../remote_session/interactive_shell.py | 289 ++++++++++++++++--
> dts/framework/remote_session/python_shell.py | 4 +
> dts/framework/remote_session/shell_pool.py | 2 +-
> .../single_active_interactive_shell.py | 275 -----------------
> dts/framework/remote_session/testpmd_shell.py | 4 +-
> 7 files changed, 275 insertions(+), 309 deletions(-)
> delete mode 100644 dts/framework/remote_session/single_active_interactive_shell.py
>
> diff --git a/dts/framework/remote_session/dpdk_app.py b/dts/framework/remote_session/dpdk_app.py
> index c9945f302d..c5aae05e02 100644
> --- a/dts/framework/remote_session/dpdk_app.py
> +++ b/dts/framework/remote_session/dpdk_app.py
> @@ -62,7 +62,7 @@ def wait_until_ready(self, end_token: str) -> None:
> Args:
> end_token: The string at the end of a line that indicates the app is ready.
> """
> - self._start_application(end_token)
> + self.start_application(end_token)
>
> def close(self) -> None:
> """Close the application.
> @@ -70,4 +70,4 @@ def close(self) -> None:
> Sends a SIGINT to close the application.
> """
> self.send_command("\x03")
> - self._close()
> + super().close()
> diff --git a/dts/framework/remote_session/dpdk_shell.py b/dts/framework/remote_session/dpdk_shell.py
> index f7ea2588ca..24a39482ce 100644
> --- a/dts/framework/remote_session/dpdk_shell.py
> +++ b/dts/framework/remote_session/dpdk_shell.py
> @@ -11,8 +11,8 @@
>
> from framework.context import get_ctx
> from framework.params.eal import EalParams
> -from framework.remote_session.single_active_interactive_shell import (
> - SingleActiveInteractiveShell,
> +from framework.remote_session.interactive_shell import (
> + InteractiveShell,
> )
> from framework.testbed_model.cpu import LogicalCoreList
>
> @@ -51,7 +51,7 @@ def compute_eal_params(
> return params
>
>
> -class DPDKShell(SingleActiveInteractiveShell, ABC):
> +class DPDKShell(InteractiveShell, ABC):
> """The base class for managing DPDK-based interactive shells.
>
> This class shouldn't be instantiated directly, but instead be extended.
> diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
> index 9ca285b604..62f9984d3b 100644
> --- a/dts/framework/remote_session/interactive_shell.py
> +++ b/dts/framework/remote_session/interactive_shell.py
> @@ -1,44 +1,281 @@
> # SPDX-License-Identifier: BSD-3-Clause
> -# Copyright(c) 2023 University of New Hampshire
> +# Copyright(c) 2024 University of New Hampshire
> # Copyright(c) 2024 Arm Limited
>
> -"""Interactive shell with manual stop/start functionality.
> +"""Common functionality for interactive shell handling.
>
> -Provides a class that doesn't require being started/stopped using a context manager and can instead
> -be started and stopped manually, or have the stopping process be handled at the time of garbage
> -collection.
> +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.
> +
> +This class is designed for applications like primary applications in DPDK where only one instance
> +of the application can be running at a given time and, for this reason, is managed using a context
> +manager. This context manager starts the application when you enter the context and cleans up the
> +application when you exit. Using a context manager for this is useful since it allows us to ensure
> +the application is cleaned up as soon as you leave the block regardless of the reason.
> +
> +The :option:`--timeout` command line argument and the :envvar:`DTS_TIMEOUT`
> +environment variable configure the timeout of getting the output from command execution.
> """
>
> -import weakref
> +from abc import ABC
> +from pathlib import PurePath
> from typing import ClassVar
>
> -from .single_active_interactive_shell import SingleActiveInteractiveShell
> +from paramiko import Channel, channel
> +from typing_extensions import Self
> +
> +from framework.exception import (
> + InteractiveCommandExecutionError,
> + InteractiveSSHSessionDeadError,
> + InteractiveSSHTimeoutError,
> +)
> +from framework.logger import DTSLogger, get_dts_logger
> +from framework.params import Params
> +from framework.settings import SETTINGS
> +from framework.testbed_model.node import Node
> +from framework.utils import MultiInheritanceBaseClass
> +
> +
> +class InteractiveShell(MultiInheritanceBaseClass, ABC):
> + """The base class for managing interactive shells.
>
> + This class shouldn't be instantiated directly, but instead be extended. It contains
> + methods for starting interactive shells as well as sending commands to these shells
> + 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.
>
> -class InteractiveShell(SingleActiveInteractiveShell):
> - """Adds manual start and stop functionality to interactive shells.
> + Interactive shells are started and stopped using a context manager. This allows for the start
> + and cleanup of the application to happen at predictable times regardless of exceptions or
> + interrupts.
>
> - Like its super-class, this class should not be instantiated directly and should instead be
> - extended. This class also provides an option for automated cleanup of the application using a
> - weakref and a finalize class. This finalize class allows for cleanup of the class at the time
> - of garbage collection and also ensures that cleanup only happens once. This way if a user
> - initiates the closing of the shell manually it is not repeated at the time of garbage
> - collection.
> + Attributes:
> + is_alive: :data:`True` if the application has started successfully, :data:`False`
> + otherwise.
> """
>
> - _finalizer: weakref.finalize
> - #: One attempt should be enough for shells which don't have to worry about other instances
> - #: closing before starting a new one.
> - _init_attempts: ClassVar[int] = 1
> + _node: Node
> + _stdin: channel.ChannelStdinFile
> + _stdout: channel.ChannelFile
> + _ssh_channel: Channel
> + _logger: DTSLogger
> + _timeout: float
> + _app_params: Params
> + _privileged: bool
> + _real_path: PurePath
> +
> + #: The number of times to try starting the application before considering it a failure.
> + _init_attempts: ClassVar[int] = 5
> +
> + #: 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. This additional newline
> + #: character is used to force the line that is currently awaiting input
> + #: into the stdout buffer so that it can be consumed and checked against
> + #: the expected prompt.
> + _command_extra_chars: ClassVar[str] = ""
> +
> + #: Path to the executable to start the interactive application.
> + path: ClassVar[PurePath]
> +
> + is_alive: bool = False
> +
> + def __init__(
> + self,
> + node: Node,
> + name: str | None = None,
> + privileged: bool = False,
> + path: PurePath | None = None,
> + app_params: Params = Params(),
> + **kwargs,
> + ) -> None:
> + """Create an SSH channel during initialization.
> +
> + Additional keyword arguments can be passed through `kwargs` if needed for fulfilling other
> + constructors in the case of multiple inheritance.
> +
> + Args:
> + node: The node on which to run start the interactive shell.
> + name: Name for the interactive shell to use for logging. This name will be appended to
> + the name of the underlying node which it is running on.
> + privileged: Enables the shell to run as superuser.
> + path: Path to the executable. If :data:`None`, then the class' path attribute is used.
> + app_params: The command line parameters to be passed to the application on startup.
> + **kwargs: Any additional arguments if any.
> + """
> + self._node = node
> + if name is None:
> + name = type(self).__name__
> + self._logger = get_dts_logger(f"{node.name}.{name}")
> + self._app_params = app_params
> + self._privileged = privileged
> + self._timeout = SETTINGS.timeout
> + # Ensure path is properly formatted for the host
> + self._update_real_path(path or self.path)
> + super().__init__(**kwargs)
> +
> + def _setup_ssh_channel(self):
> + self._ssh_channel = self._node.main_session.interactive_session.session.invoke_shell()
> + self._stdin = self._ssh_channel.makefile_stdin("w")
> + self._stdout = self._ssh_channel.makefile("r")
> + self._ssh_channel.settimeout(self._timeout)
> + self._ssh_channel.set_combine_stderr(True) # combines stdout and stderr streams
> +
> + def _make_start_command(self) -> str:
> + """Makes the command that starts the interactive shell."""
> + start_command = f"{self._real_path} {self._app_params or ''}"
> + if self._privileged:
> + start_command = self._node.main_session._get_privileged_command(start_command)
> + return start_command
> +
> + def start_application(self, prompt: str | None = None) -> None:
> + """Starts a new interactive application based on the path to the app.
> +
> + This method is often overridden by subclasses as their process for starting may look
> + different. Initialization of the shell on the host can be retried up to
> + `self._init_attempts` - 1 times. This is done because some DPDK applications need slightly
> + more time after exiting their script to clean up EAL before others can start.
>
> - def start_application(self) -> None:
> - """Start the application.
> + Args:
> + prompt: When starting up the application, expect this string at the end of stdout when
> + the application is ready. If :data:`None`, the class' default prompt will be used.
>
> - After the application has started, use :class:`weakref.finalize` to manage cleanup.
> + Raises:
> + InteractiveCommandExecutionError: If the application fails to start within the allotted
> + number of retries.
> """
> - self._start_application()
> - self._finalizer = weakref.finalize(self, self._close)
> + self._setup_ssh_channel()
> + self._ssh_channel.settimeout(5)
> + start_command = self._make_start_command()
> + self.is_alive = True
> + for attempt in range(self._init_attempts):
> + try:
> + self.send_command(start_command, prompt)
> + break
> + except InteractiveSSHTimeoutError:
> + self._logger.info(
> + f"Interactive shell failed to start (attempt {attempt+1} out of "
> + f"{self._init_attempts})"
> + )
> + else:
> + self._ssh_channel.settimeout(self._timeout)
> + self.is_alive = False # update state on failure to start
> + raise InteractiveCommandExecutionError("Failed to start application.")
> + self._ssh_channel.settimeout(self._timeout)
> +
> + def send_command(
> + self, command: str, prompt: str | None = None, skip_first_line: bool = False
> + ) -> str:
> + """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.
> +
> + 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.
> + skip_first_line: Skip the first line when capturing the output.
> +
> + Returns:
> + All output in the buffer before expected string.
> +
> + Raises:
> + InteractiveCommandExecutionError: If attempting to send a command to a shell that is
> + not currently running.
> + InteractiveSSHSessionDeadError: The session died while executing the command.
> + InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
> + the output before the timeout.
> + """
> + if not self.is_alive:
> + raise InteractiveCommandExecutionError(
> + f"Cannot send command {command} to application because the shell is not running."
> + )
> + self._logger.info(f"Sending: '{command}'")
> + if prompt is None:
> + prompt = self._default_prompt
> + out: str = ""
> + try:
> + self._stdin.write(f"{command}{self._command_extra_chars}\n")
> + self._stdin.flush()
> + for line in self._stdout:
> + if skip_first_line:
> + skip_first_line = False
> + continue
> + if line.rstrip().endswith(prompt):
> + break
> + out += line
> + except TimeoutError as e:
> + self._logger.exception(e)
> + self._logger.debug(
> + f"Prompt ({prompt}) was not found in output from command before timeout."
> + )
> + raise InteractiveSSHTimeoutError(command) from e
> + except OSError as e:
> + self._logger.exception(e)
> + raise InteractiveSSHSessionDeadError(
> + self._node.main_session.interactive_session.hostname
> + ) from e
> + finally:
> + self._logger.debug(f"Got output: {out}")
> + return out
>
> def close(self) -> None:
> - """Free all resources using :class:`weakref.finalize`."""
> - self._finalizer()
> + """Close the shell.
> +
> + Raises:
> + InteractiveSSHTimeoutError: If the shell failed to exit within the set timeout.
> + """
> + try:
> + # Ensure the primary application has terminated via readiness of 'stdout'.
> + if self._ssh_channel.recv_ready():
> + self._ssh_channel.recv(1) # 'Waits' for a single byte to enter 'stdout' buffer.
> + except TimeoutError as e:
> + self._logger.exception(e)
> + self._logger.debug("Application failed to exit before set timeout.")
> + raise InteractiveSSHTimeoutError("Application 'exit' command") from e
> + self._ssh_channel.close()
> +
> + def _update_real_path(self, path: PurePath) -> None:
> + """Updates the interactive shell's real path used at command line."""
> + self._real_path = self._node.main_session.join_remote_path(path)
> +
> + def __enter__(self) -> Self:
> + """Enter the context block.
> +
> + Upon entering a context block with this class, the desired behavior is to create the
> + channel for the application to use, and then start the application.
> +
> + Returns:
> + Reference to the object for the application after it has been started.
> + """
> + self.start_application()
> + return self
> +
> + def __exit__(self, *_) -> None:
> + """Exit the context block.
> +
> + Upon exiting a context block with this class, we want to ensure that the instance of the
> + application is explicitly closed and properly cleaned up using its close method. Note that
> + because this method returns :data:`None` if an exception was raised within the block, it is
> + not handled and will be re-raised after the application is closed.
> +
> + The desired behavior is to close the application regardless of the reason for exiting the
> + context and then recreate that reason afterwards. All method arguments are ignored for
> + this reason.
> + """
> + self.close()
> diff --git a/dts/framework/remote_session/python_shell.py b/dts/framework/remote_session/python_shell.py
> index 9d4abab12c..17e6c2559d 100644
> --- a/dts/framework/remote_session/python_shell.py
> +++ b/dts/framework/remote_session/python_shell.py
> @@ -29,3 +29,7 @@ class PythonShell(InteractiveShell):
>
> #: The Python executable.
> path: ClassVar[PurePath] = PurePath("python3")
> +
> + def close(self):
> + """Close Python shell."""
> + return super().close()
> diff --git a/dts/framework/remote_session/shell_pool.py b/dts/framework/remote_session/shell_pool.py
> index 173aa8fd36..da956950d5 100644
> --- a/dts/framework/remote_session/shell_pool.py
> +++ b/dts/framework/remote_session/shell_pool.py
> @@ -90,7 +90,7 @@ def terminate_current_pool(self):
> for shell in self._pools.pop():
> self._logger.debug(f"Closing shell {shell} in shell pool level {current_pool_level}.")
> try:
> - shell._close()
> + shell.close()
> except Exception as e:
> self._logger.error(f"An exception has occurred while closing shell {shell}:")
> self._logger.exception(e)
> diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
> deleted file mode 100644
> index 2257b6156b..0000000000
> --- a/dts/framework/remote_session/single_active_interactive_shell.py
> +++ /dev/null
> @@ -1,275 +0,0 @@
> -# SPDX-License-Identifier: BSD-3-Clause
> -# Copyright(c) 2024 University of New Hampshire
> -
> -"""Common functionality for interactive shell handling.
> -
> -The base class, :class:`SingleActiveInteractiveShell`, 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.
> -
> -This class is designed for applications like primary applications in DPDK where only one instance
> -of the application can be running at a given time and, for this reason, is managed using a context
> -manager. This context manager starts the application when you enter the context and cleans up the
> -application when you exit. Using a context manager for this is useful since it allows us to ensure
> -the application is cleaned up as soon as you leave the block regardless of the reason.
> -
> -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 ClassVar
> -
> -from paramiko import Channel, channel
> -from typing_extensions import Self
> -
> -from framework.exception import (
> - InteractiveCommandExecutionError,
> - InteractiveSSHSessionDeadError,
> - InteractiveSSHTimeoutError,
> -)
> -from framework.logger import DTSLogger, get_dts_logger
> -from framework.params import Params
> -from framework.settings import SETTINGS
> -from framework.testbed_model.node import Node
> -from framework.utils import MultiInheritanceBaseClass
> -
> -
> -class SingleActiveInteractiveShell(MultiInheritanceBaseClass, ABC):
> - """The base class for managing interactive shells.
> -
> - This class shouldn't be instantiated directly, but instead be extended. It contains
> - methods for starting interactive shells as well as sending commands to these shells
> - 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.
> -
> - Interactive shells are started and stopped using a context manager. This allows for the start
> - and cleanup of the application to happen at predictable times regardless of exceptions or
> - interrupts.
> -
> - Attributes:
> - is_alive: :data:`True` if the application has started successfully, :data:`False`
> - otherwise.
> - """
> -
> - _node: Node
> - _stdin: channel.ChannelStdinFile
> - _stdout: channel.ChannelFile
> - _ssh_channel: Channel
> - _logger: DTSLogger
> - _timeout: float
> - _app_params: Params
> - _privileged: bool
> - _real_path: PurePath
> -
> - #: The number of times to try starting the application before considering it a failure.
> - _init_attempts: ClassVar[int] = 5
> -
> - #: 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. This additional newline
> - #: character is used to force the line that is currently awaiting input
> - #: into the stdout buffer so that it can be consumed and checked against
> - #: the expected prompt.
> - _command_extra_chars: ClassVar[str] = ""
> -
> - #: Path to the executable to start the interactive application.
> - path: ClassVar[PurePath]
> -
> - is_alive: bool = False
> -
> - def __init__(
> - self,
> - node: Node,
> - name: str | None = None,
> - privileged: bool = False,
> - path: PurePath | None = None,
> - app_params: Params = Params(),
> - **kwargs,
> - ) -> None:
> - """Create an SSH channel during initialization.
> -
> - Additional keyword arguments can be passed through `kwargs` if needed for fulfilling other
> - constructors in the case of multiple inheritance.
> -
> - Args:
> - node: The node on which to run start the interactive shell.
> - name: Name for the interactive shell to use for logging. This name will be appended to
> - the name of the underlying node which it is running on.
> - privileged: Enables the shell to run as superuser.
> - path: Path to the executable. If :data:`None`, then the class' path attribute is used.
> - app_params: The command line parameters to be passed to the application on startup.
> - **kwargs: Any additional arguments if any.
> - """
> - self._node = node
> - if name is None:
> - name = type(self).__name__
> - self._logger = get_dts_logger(f"{node.name}.{name}")
> - self._app_params = app_params
> - self._privileged = privileged
> - self._timeout = SETTINGS.timeout
> - # Ensure path is properly formatted for the host
> - self._update_real_path(path or self.path)
> - super().__init__(**kwargs)
> -
> - def _setup_ssh_channel(self):
> - self._ssh_channel = self._node.main_session.interactive_session.session.invoke_shell()
> - self._stdin = self._ssh_channel.makefile_stdin("w")
> - self._stdout = self._ssh_channel.makefile("r")
> - self._ssh_channel.settimeout(self._timeout)
> - self._ssh_channel.set_combine_stderr(True) # combines stdout and stderr streams
> -
> - def _make_start_command(self) -> str:
> - """Makes the command that starts the interactive shell."""
> - start_command = f"{self._real_path} {self._app_params or ''}"
> - if self._privileged:
> - start_command = self._node.main_session._get_privileged_command(start_command)
> - return start_command
> -
> - def _start_application(self, prompt: str | None = None) -> None:
> - """Starts a new interactive application based on the path to the app.
> -
> - This method is often overridden by subclasses as their process for starting may look
> - different. Initialization of the shell on the host can be retried up to
> - `self._init_attempts` - 1 times. This is done because some DPDK applications need slightly
> - more time after exiting their script to clean up EAL before others can start.
> -
> - Args:
> - prompt: When starting up the application, expect this string at the end of stdout when
> - the application is ready. If :data:`None`, the class' default prompt will be used.
> -
> - Raises:
> - InteractiveCommandExecutionError: If the application fails to start within the allotted
> - number of retries.
> - """
> - self._setup_ssh_channel()
> - self._ssh_channel.settimeout(5)
> - start_command = self._make_start_command()
> - self.is_alive = True
> - for attempt in range(self._init_attempts):
> - try:
> - self.send_command(start_command, prompt)
> - break
> - except InteractiveSSHTimeoutError:
> - self._logger.info(
> - f"Interactive shell failed to start (attempt {attempt+1} out of "
> - f"{self._init_attempts})"
> - )
> - else:
> - self._ssh_channel.settimeout(self._timeout)
> - self.is_alive = False # update state on failure to start
> - raise InteractiveCommandExecutionError("Failed to start application.")
> - self._ssh_channel.settimeout(self._timeout)
> -
> - def send_command(
> - self, command: str, prompt: str | None = None, skip_first_line: bool = False
> - ) -> str:
> - """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.
> -
> - 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.
> - skip_first_line: Skip the first line when capturing the output.
> -
> - Returns:
> - All output in the buffer before expected string.
> -
> - Raises:
> - InteractiveCommandExecutionError: If attempting to send a command to a shell that is
> - not currently running.
> - InteractiveSSHSessionDeadError: The session died while executing the command.
> - InteractiveSSHTimeoutError: If command was sent but prompt could not be found in
> - the output before the timeout.
> - """
> - if not self.is_alive:
> - raise InteractiveCommandExecutionError(
> - f"Cannot send command {command} to application because the shell is not running."
> - )
> - self._logger.info(f"Sending: '{command}'")
> - if prompt is None:
> - prompt = self._default_prompt
> - out: str = ""
> - try:
> - self._stdin.write(f"{command}{self._command_extra_chars}\n")
> - self._stdin.flush()
> - for line in self._stdout:
> - if skip_first_line:
> - skip_first_line = False
> - continue
> - if line.rstrip().endswith(prompt):
> - break
> - out += line
> - except TimeoutError as e:
> - self._logger.exception(e)
> - self._logger.debug(
> - f"Prompt ({prompt}) was not found in output from command before timeout."
> - )
> - raise InteractiveSSHTimeoutError(command) from e
> - except OSError as e:
> - self._logger.exception(e)
> - raise InteractiveSSHSessionDeadError(
> - self._node.main_session.interactive_session.hostname
> - ) from e
> - finally:
> - self._logger.debug(f"Got output: {out}")
> - return out
> -
> - def _close(self) -> None:
> - try:
> - # Ensure the primary application has terminated via readiness of 'stdout'.
> - if self._ssh_channel.recv_ready():
> - self._ssh_channel.recv(1) # 'Waits' for a single byte to enter 'stdout' buffer.
> - except TimeoutError as e:
> - self._logger.exception(e)
> - self._logger.debug("Application failed to exit before set timeout.")
> - raise InteractiveSSHTimeoutError("Application 'exit' command") from e
> - self._ssh_channel.close()
> -
> - def _update_real_path(self, path: PurePath) -> None:
> - """Updates the interactive shell's real path used at command line."""
> - self._real_path = self._node.main_session.join_remote_path(path)
> -
> - def __enter__(self) -> Self:
> - """Enter the context block.
> -
> - Upon entering a context block with this class, the desired behavior is to create the
> - channel for the application to use, and then start the application.
> -
> - Returns:
> - Reference to the object for the application after it has been started.
> - """
> - self._start_application()
> - return self
> -
> - def __exit__(self, *_) -> None:
> - """Exit the context block.
> -
> - Upon exiting a context block with this class, we want to ensure that the instance of the
> - application is explicitly closed and properly cleaned up using its close method. Note that
> - because this method returns :data:`None` if an exception was raised within the block, it is
> - not handled and will be re-raised after the application is closed.
> -
> - The desired behavior is to close the application regardless of the reason for exiting the
> - context and then recreate that reason afterwards. All method arguments are ignored for
> - this reason.
> - """
> - self._close()
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
> index db1bfaa9d1..b83b0c82a0 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -2312,11 +2312,11 @@ def rx_vxlan(self, vxlan_id: int, port_id: int, enable: bool, verify: bool = Tru
> self._logger.debug(f"Failed to set VXLAN:\n{vxlan_output}")
> raise InteractiveCommandExecutionError(f"Failed to set VXLAN:\n{vxlan_output}")
>
> - def _close(self) -> None:
> + def close(self) -> None:
> """Overrides :meth:`~.interactive_shell.close`."""
> self.stop()
> self.send_command("quit", "Bye...")
> - return super()._close()
> + return super().close()
>
> """
> ====== Capability retrieval methods ======
> --
> 2.43.0
>
next prev parent reply other threads:[~2025-04-25 18:18 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-20 17:23 [RFC PATCH 0/2] dts: add basic scope to improve shell handling Luca Vizzarro
2024-12-20 17:24 ` [RFC PATCH 1/2] dts: add scoping and shell registration to Node Luca Vizzarro
2024-12-20 17:24 ` [RFC PATCH 2/2] dts: revert back shell split Luca Vizzarro
2025-03-14 13:18 ` [PATCH v2 0/7] dts: shell improvements Luca Vizzarro
2025-03-14 13:18 ` [PATCH v2 1/7] dts: escape single quotes Luca Vizzarro
2025-04-24 17:55 ` Dean Marx
2025-04-25 2:20 ` Patrick Robb
2025-04-25 16:15 ` Nicholas Pratte
2025-03-14 13:18 ` [PATCH v2 2/7] dts: add blocking dpdk app class Luca Vizzarro
2025-04-24 18:01 ` Dean Marx
2025-04-25 2:38 ` Patrick Robb
2025-04-25 16:56 ` Nicholas Pratte
2025-03-14 13:18 ` [PATCH v2 3/7] dts: add shells pool Luca Vizzarro
2025-04-24 18:15 ` Dean Marx
2025-04-25 3:06 ` Patrick Robb
2025-03-14 13:18 ` [PATCH v2 4/7] dts: revert back to a single InteractiveShell Luca Vizzarro
2025-04-24 18:22 ` Dean Marx
2025-04-25 3:26 ` Patrick Robb
2025-04-25 18:17 ` Nicholas Pratte [this message]
2025-03-14 13:18 ` [PATCH v2 5/7] dts: make shells path dynamic Luca Vizzarro
2025-04-24 18:29 ` Dean Marx
2025-04-25 3:31 ` Patrick Robb
2025-04-25 18:35 ` Nicholas Pratte
2025-03-14 13:18 ` [PATCH v2 6/7] dts: remove multi-inheritance classes Luca Vizzarro
2025-04-24 18:36 ` Dean Marx
2025-04-25 3:36 ` Patrick Robb
2025-04-25 18:39 ` Nicholas Pratte
2025-03-14 13:18 ` [PATCH v2 7/7] dts: enable shell pooling Luca Vizzarro
2025-04-24 18:40 ` Dean Marx
2025-04-25 3:41 ` Patrick Robb
2025-04-25 18:50 ` Nicholas Pratte
2025-04-25 3:49 ` [PATCH v2 0/7] dts: shell improvements Patrick Robb
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKXZ7eis5F0qQr-xe+S-HjXQFknUXuNuBnTEukDng_6YVAcvaQ@mail.gmail.com \
--to=npratte@iol.unh.edu \
--cc=dev@dpdk.org \
--cc=luca.vizzarro@arm.com \
--cc=paul.szczepanek@arm.com \
--cc=probb@iol.unh.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).