[v4,3/8] dts: refactor build and node info classes
Checks
Commit Message
The DPDKBuildInfo and NodeInfo classes, representing information
gathered in runtime, were erroneously placed in the configuration
package. This moves them in more appropriate modules.
NodeInfo, specifically, ia moved to os_session instead of node mostly
as a consequence of circular dependencies. And given os_session is the
top-most module to reference it, it appears to be the most suitable
place outside of node.
Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
---
dts/framework/config/__init__.py | 31 --------------------
dts/framework/test_result.py | 4 ++-
dts/framework/testbed_model/os_session.py | 21 ++++++++++++-
dts/framework/testbed_model/posix_session.py | 4 +--
dts/framework/testbed_model/sut_node.py | 18 ++++++++++--
5 files changed, 40 insertions(+), 38 deletions(-)
Comments
It took me a second to appreciate what the goal of separating this is,
but it makes complete sense to me now. This was an oversight on my end
as well when I was working on the config changes in once of the
patches I was assigned. Interestingly enough, I ran into a similar
problem with circular dependencies a long time ago when I was
attempting to do some 'arch' discovery changes, which I still intend
to implement. I think putting the node config in the os_session
component makes sense from a readability standpoint as well.
On Mon, Oct 28, 2024 at 1:51 PM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> The DPDKBuildInfo and NodeInfo classes, representing information
> gathered in runtime, were erroneously placed in the configuration
> package. This moves them in more appropriate modules.
>
> NodeInfo, specifically, ia moved to os_session instead of node mostly
Small typo here, change 'ia' to 'is'.
> as a consequence of circular dependencies. And given os_session is the
> top-most module to reference it, it appears to be the most suitable
> place outside of node.
As I said, this makes sense to me, but I wonder if it might make sense
to change 'NodeInfo' to 'OSSessionInfo' or something like that. I'd
imagine that if any attributes were to be tacked on in the future they
would probably be os related, but maybe there would be system
information, and in this case "OSSessionInfo" might be a good middle
ground. There are existing changes that I've done where arch is
discovered during runtime, and this could probably be placed in this
'NodeInfo' class as well when I get around to revising it. My only
concern is whether or not having "NodeConfiguration" and "NodeInfo"
classes floating around might make the framework more confusing to
read.
<snip>
On 31/10/2024 20:16, Nicholas Pratte wrote:
> On Mon, Oct 28, 2024 at 1:51 PM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>>
>> The DPDKBuildInfo and NodeInfo classes, representing information
>> gathered in runtime, were erroneously placed in the configuration
>> package. This moves them in more appropriate modules.
>>
>> NodeInfo, specifically, ia moved to os_session instead of node mostly
>
> Small typo here, change 'ia' to 'is'.
Once again, great catch!
>> as a consequence of circular dependencies. And given os_session is the
>> top-most module to reference it, it appears to be the most suitable
>> place outside of node.
>
> As I said, this makes sense to me, but I wonder if it might make sense
> to change 'NodeInfo' to 'OSSessionInfo' or something like that. I'd
> imagine that if any attributes were to be tacked on in the future they
> would probably be os related, but maybe there would be system
> information, and in this case "OSSessionInfo" might be a good middle
> ground. There are existing changes that I've done where arch is
> discovered during runtime, and this could probably be placed in this
> 'NodeInfo' class as well when I get around to revising it. My only
> concern is whether or not having "NodeConfiguration" and "NodeInfo"
> classes floating around might make the framework more confusing to
> read.
You make an excellent point, I didn't think of it too much. This is a
great suggestion, will apply it.
@@ -318,24 +318,6 @@ class TGNodeConfiguration(NodeConfiguration):
traffic_generator: TrafficGeneratorConfig
-@dataclass(slots=True, frozen=True)
-class NodeInfo:
- """Supplemental node information.
-
- Attributes:
- os_name: The name of the running operating system of
- the :class:`~framework.testbed_model.node.Node`.
- os_version: The version of the running operating system of
- the :class:`~framework.testbed_model.node.Node`.
- kernel_version: The kernel version of the running operating system of
- the :class:`~framework.testbed_model.node.Node`.
- """
-
- os_name: str
- os_version: str
- kernel_version: str
-
-
@dataclass(slots=True, frozen=True)
class DPDKBuildConfiguration:
"""DPDK build configuration.
@@ -493,19 +475,6 @@ def from_dict(cls, d: DPDKConfigurationDict) -> Self:
)
-@dataclass(slots=True, frozen=True)
-class DPDKBuildInfo:
- """Various versions and other information about a DPDK build.
-
- Attributes:
- dpdk_version: The DPDK version that was built.
- compiler_version: The version of the compiler used to build DPDK.
- """
-
- dpdk_version: str | None
- compiler_version: str | None
-
-
@dataclass(slots=True, frozen=True)
class TestSuiteConfig:
"""Test suite configuration.
@@ -30,11 +30,13 @@
from framework.testbed_model.capability import Capability
-from .config import DPDKBuildInfo, NodeInfo, TestRunConfiguration, TestSuiteConfig
+from .config import TestRunConfiguration, TestSuiteConfig
from .exception import DTSError, ErrorSeverity
from .logger import DTSLogger
from .settings import SETTINGS
from .test_suite import TestCase, TestSuite
+from .testbed_model.os_session import NodeInfo
+from .testbed_model.sut_node import DPDKBuildInfo
@dataclass(slots=True, frozen=True)
@@ -24,11 +24,12 @@
"""
from abc import ABC, abstractmethod
from collections.abc import Iterable
+from dataclasses import dataclass
from ipaddress import IPv4Interface, IPv6Interface
from pathlib import Path, PurePath, PurePosixPath
from typing import Union
-from framework.config import Architecture, NodeConfiguration, NodeInfo
+from framework.config import Architecture, NodeConfiguration
from framework.logger import DTSLogger
from framework.remote_session import (
InteractiveRemoteSession,
@@ -44,6 +45,24 @@
from .port import Port
+@dataclass(slots=True, frozen=True)
+class NodeInfo:
+ """Supplemental node information.
+
+ Attributes:
+ os_name: The name of the running operating system of
+ the :class:`~framework.testbed_model.node.Node`.
+ os_version: The version of the running operating system of
+ the :class:`~framework.testbed_model.node.Node`.
+ kernel_version: The kernel version of the running operating system of
+ the :class:`~framework.testbed_model.node.Node`.
+ """
+
+ os_name: str
+ os_version: str
+ kernel_version: str
+
+
class OSSession(ABC):
"""OS-unaware to OS-aware translation API definition.
@@ -15,7 +15,7 @@
from collections.abc import Iterable
from pathlib import Path, PurePath, PurePosixPath
-from framework.config import Architecture, NodeInfo
+from framework.config import Architecture
from framework.exception import DPDKBuildError, RemoteCommandExecutionError
from framework.settings import SETTINGS
from framework.utils import (
@@ -26,7 +26,7 @@
extract_tarball,
)
-from .os_session import OSSession
+from .os_session import NodeInfo, OSSession
class PosixSession(OSSession):
@@ -14,13 +14,12 @@
import os
import time
+from dataclasses import dataclass
from pathlib import PurePath
from framework.config import (
DPDKBuildConfiguration,
- DPDKBuildInfo,
DPDKLocation,
- NodeInfo,
SutNodeConfiguration,
TestRunConfiguration,
)
@@ -30,10 +29,23 @@
from framework.utils import MesonArgs, TarCompressionFormat
from .node import Node
-from .os_session import OSSession
+from .os_session import NodeInfo, OSSession
from .virtual_device import VirtualDevice
+@dataclass(slots=True, frozen=True)
+class DPDKBuildInfo:
+ """Various versions and other information about a DPDK build.
+
+ Attributes:
+ dpdk_version: The DPDK version that was built.
+ compiler_version: The version of the compiler used to build DPDK.
+ """
+
+ dpdk_version: str | None
+ compiler_version: str | None
+
+
class SutNode(Node):
"""The system under test node.