The subject line should start with a lowercase character and it's also
over the character limit. This fits and basically says the same:
use PythonShell for Scapy instead of XML-RPC
Scapy TG didn't fit but I think the above still works.
diff --git a/dts/framework/remote_session/single_active_interactive_shell.py
b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -93,9 +94,13 @@ def __init__(
timeout: float = SETTINGS.timeout,
app_params: Params = Params(),
name: str | None = None,
+ **kwargs,
) -> None:
"""Create an SSH channel during initialization.
+ Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
key-word -> keyword
+ constructors in the case of multiple-inheritance.
I also didn't notice this hyphen, it shouldn't be there.
These hyphens are in a lot of places, I don't if I caught them all.
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py
b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,311 +6,150 @@
A traffic generator used for functional testing, implemented with
`the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG
node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG
node.
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
-in an interactive remote Python SSH session. The communication with the server
is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends
:class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the
interactive shell.
Since this only mentions PythonShell, people could understand that's the
only thing the tg extends. We should either add a qualifier such as
additionally extends or just explicitly say it also extends the
capturing TG (which is also useful information as it tells us which kind
of TG it is).
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+ """Provides access to scapy functions on a traffic generator node.
+ This class extends the base with remote execution of scapy functions. All
methods for
+ processing packets are implemented using an underlying
+ :class:`framework.remote_session.python_shell.PythonShell` which imports
the Scapy library.
+ Because of the dual-inheritance, this class has both methods that wrap
scapy commands sent into
The hyphen doesn't sound right. And maybe double would be a better term.
I understand dual to mean two of the same and that doesn't fit for me as
well.
+ the shell and methods that run locally to fulfill traffic generation
needs. To help make a
+ clear distinction between the two, the names of the methods that wrap the
logic of the
+ underlying shell should be prepended with "shell".
I think there would be value in explicitly saying that the shell runs on
the TG node.
+ Note that the order of inheritance is important for this class. In order
to instantiate this
+ class, the abstract methods of
:class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
+ must be implemented. Since some of these methods are implemented in the
underlying interactive
+ shell, according to Python's Method Resolution Order (MRO), the
interactive shell must come
+ first.
I didn't notice this before. Is this because of the close() method? Do
we need to add any special steps to close the TG? Closing the
interactive session should be enough, but I wanted to check with you.
+ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig,
**kwargs):
+ """Extend the constructor with Scapy TG specifics.
- def __init__(self, *args, **kwargs):
- """Extend the XML-RPC server initialization.
+ Initializes both the traffic generator and the interactive shell used
to handle Scapy
+ functions. The interactive shell will be started on `tg_node`. The
additional key-word
+ arguments in `kwargs` are used to pass into the constructor for the
interactive shell.
Args:
- args: The positional arguments that will be passed to the
superclass's constructor.
- kwargs: The keyword arguments that will be passed to the
superclass's constructor.
- The `allow_none` argument will be set to :data:`True`.
+ tg_node: The node where the traffic generator resides.
+ config: The traffic generator's test run configuration.
+ kwargs: Keyword arguments all traffic generators must support in
order to allow for
+ multiple-inheritance.
I'm not sure why all and why they must support them. We can just say the
supported keyword arguments corresdpond to the parameters of
:meth:`PythonShell.__init__`.
Also, multiple-inheritance -> multiple inheritance
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index 4ce1148706..176d5e9065 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -16,23 +16,29 @@
from framework.logger import DTSLogger, get_dts_logger
from framework.testbed_model.node import Node
from framework.testbed_model.port import Port
-from framework.utils import get_packet_summaries
+from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
-class TrafficGenerator(ABC):
+class TrafficGenerator(MultiInheritanceBaseClass, ABC):
"""The base traffic generator.
Exposes the common public methods of all traffic generators and defines private methods
- that must implement the traffic generation logic in subclasses.
+ that must implement the traffic generation logic in subclasses. This class
also extends from
+ :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the
ability to inherit
+ from multiple classes to fulfil the traffic generating functionality
without breaking
+ single-inheritance.
Hyphen again.
"""
_config: TrafficGeneratorConfig
_tg_node: Node
_logger: DTSLogger
- def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
+ def __init__(self, tg_node: Node, config: TrafficGeneratorConfig,
**kwargs):
"""Initialize the traffic generator.
+ Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
+ constructors in the case of multiple-inheritance.
+
keyword, multiple inheritance
