Acked-by: Jeremy Spweock On Thu, May 11, 2023 at 5:14 AM Juraj Linkeš wrote: > WIP: only one module is reformatted to serve as a demonstration. > > The google format is documented here [0]. > > [0]: https://google.github.io/styleguide/pyguide.html > > Signed-off-by: Juraj Linkeš > --- > dts/framework/testbed_model/node.py | 152 +++++++++++++++++++--------- > 1 file changed, 103 insertions(+), 49 deletions(-) > > diff --git a/dts/framework/testbed_model/node.py > b/dts/framework/testbed_model/node.py > index 90467981c3..ad8ef442af 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -3,8 +3,13 @@ > # Copyright(c) 2022-2023 PANTHEON.tech s.r.o. > # Copyright(c) 2022-2023 University of New Hampshire > > -""" > -A node is a generic host that DTS connects to and manages. > +"""Common functionality for node management. > + > +There's a base class, Node, that's supposed to be extended by other > classes > +with functionality specific to that node type. > +The only part that can be used standalone is the Node.skip_setup static > method, > +which is a decorator used to skip method execution > +if skip_setup is passed by the user on the cmdline or in an env variable. > """ > > from typing import Any, Callable > @@ -26,10 +31,25 @@ > > > class Node(object): > - """ > - Basic class for node management. This class implements methods that > - manage a node, such as information gathering (of CPU/PCI/NIC) and > - environment setup. > + """The base class for node management. > + > + It shouldn't be instantiated, but rather extended. > + It implements common methods to manage any node: > + > + * connection to the node > + * information gathering of CPU > + * hugepages setup > + > + Arguments: > + node_config: The config from the input configuration file. > + > + Attributes: > + main_session: The primary OS-agnostic remote session used > + to communicate with the node. > + config: The configuration used to create the node. > + name: The name of the node. > + lcores: The list of logical cores that DTS can use on the node. > + It's derived from logical cores present on the node and user > configuration. > """ > > main_session: OSSession > @@ -56,65 +76,89 @@ def __init__(self, node_config: NodeConfiguration): > self._logger.info(f"Created node: {self.name}") > > def set_up_execution(self, execution_config: ExecutionConfiguration) > -> None: > - """ > - Perform the execution setup that will be done for each execution > - this node is part of. > + """Execution setup steps. > + > + Configure hugepages and call self._set_up_execution where > + the rest of the configuration steps (if any) are implemented. > + > + Args: > + execution_config: The execution configuration according to > which > + the setup steps will be taken. > """ > self._setup_hugepages() > self._set_up_execution(execution_config) > > def _set_up_execution(self, execution_config: ExecutionConfiguration) > -> None: > - """ > - This method exists to be optionally overwritten by derived > classes and > - is not decorated so that the derived class doesn't have to use > the decorator. > + """Optional additional execution setup steps for derived classes. > + > + Derived classes should overwrite this > + if they want to add additional execution setup steps. > """ > > def tear_down_execution(self) -> None: > - """ > - Perform the execution teardown that will be done after each > execution > - this node is part of concludes. > + """Execution teardown steps. > + > + There are currently no common execution teardown steps > + common to all DTS node types. > """ > self._tear_down_execution() > > def _tear_down_execution(self) -> None: > - """ > - This method exists to be optionally overwritten by derived > classes and > - is not decorated so that the derived class doesn't have to use > the decorator. > + """Optional additional execution teardown steps for derived > classes. > + > + Derived classes should overwrite this > + if they want to add additional execution teardown steps. > """ > > def set_up_build_target( > self, build_target_config: BuildTargetConfiguration > ) -> None: > - """ > - Perform the build target setup that will be done for each build > target > - tested on this node. > + """Build target setup steps. > + > + There are currently no common build target setup steps > + common to all DTS node types. > + > + Args: > + build_target_config: The build target configuration according > to which > + the setup steps will be taken. > """ > self._set_up_build_target(build_target_config) > > def _set_up_build_target( > self, build_target_config: BuildTargetConfiguration > ) -> None: > - """ > - This method exists to be optionally overwritten by derived > classes and > - is not decorated so that the derived class doesn't have to use > the decorator. > + """Optional additional build target setup steps for derived > classes. > + > + Derived classes should optionally overwrite this > + if they want to add additional build target setup steps. > """ > > def tear_down_build_target(self) -> None: > - """ > - Perform the build target teardown that will be done after each > build target > - tested on this node. > + """Build target teardown steps. > + > + There are currently no common build target teardown steps > + common to all DTS node types. > """ > self._tear_down_build_target() > > def _tear_down_build_target(self) -> None: > - """ > - This method exists to be optionally overwritten by derived > classes and > - is not decorated so that the derived class doesn't have to use > the decorator. > + """Optional additional build target teardown steps for derived > classes. > + > + Derived classes should overwrite this > + if they want to add additional build target teardown steps. > """ > > def create_session(self, name: str) -> OSSession: > - """ > - Create and return a new OSSession tailored to the remote OS. > + """Create and return a new OS-agnostic remote session. > + > + The returned session won't be used by the object creating it. > + Will be cleaned up automatically. > + > + Args: > + name: The name of the session. > + > + Returns: > + A new OS-agnostic remote session. > """ > session_name = f"{self.name} {name}" > connection = create_session( > @@ -130,14 +174,24 @@ def filter_lcores( > filter_specifier: LogicalCoreCount | LogicalCoreList, > ascending: bool = True, > ) -> list[LogicalCore]: > - """ > - Filter the LogicalCores found on the Node according to > - a LogicalCoreCount or a LogicalCoreList. > + """Filter the node's logical cores that DTS can use. > > - If ascending is True, use cores with the lowest numerical id first > - and continue in ascending order. If False, start with the highest > - id and continue in descending order. This ordering affects which > - sockets to consider first as well. > + Logical cores that DTS can use are ones that are present on the > node, > + but filtered according to user config. > + The filter_specifier will filter cores from those logical cores. > + > + Args: > + filter_specifier: Two different filters can be used, one that > specifies > + the number of logical cores per core, cores per socket and > + the number of sockets, > + the other that specifies a logical core list. > + ascending: If True, use cores with the lowest numerical id > first > + and continue in ascending order. If False, start with the > highest > + id and continue in descending order. This ordering > affects which > + sockets to consider first as well. > + > + Returns: > + A list of logical cores. > """ > self._logger.debug(f"Filtering {filter_specifier} from > {self.lcores}.") > return lcore_filter( > @@ -147,17 +201,14 @@ def filter_lcores( > ).filter() > > def _get_remote_cpus(self) -> None: > - """ > - Scan CPUs in the remote OS and store a list of LogicalCores. > - """ > + """Scan CPUs in the remote OS and store a list of LogicalCores.""" > self._logger.info("Getting CPU information.") > self.lcores = > self.main_session.get_remote_cpus(self.config.use_first_core) > > def _setup_hugepages(self): > - """ > - Setup hugepages on the Node. Different architectures can supply > different > - amounts of memory for hugepages and numa-based hugepage > allocation may need > - to be considered. > + """Setup hugepages on the Node. > + > + Configure the hugepages only if they're specified in user > configuration. > """ > if self.config.hugepages: > self.main_session.setup_hugepages( > @@ -165,9 +216,7 @@ def _setup_hugepages(self): > ) > > def close(self) -> None: > - """ > - Close all connections and free other resources. > - """ > + """Close all connections and free other resources.""" > if self.main_session: > self.main_session.close() > for session in self._other_sessions: > @@ -176,6 +225,11 @@ def close(self) -> None: > > @staticmethod > def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]: > + """A decorator that skips the decorated function. > + > + When used, the decorator executes an empty lambda function > + instead of the decorated function. > + """ > if SETTINGS.skip_setup: > return lambda *args: None > else: > -- > 2.30.2 > >