From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F408C432D9; Wed, 8 Nov 2023 18:14:14 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D0A7C402BB; Wed, 8 Nov 2023 18:14:14 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 10FEA402A7 for ; Wed, 8 Nov 2023 18:14:13 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1FDAF1476; Wed, 8 Nov 2023 09:14:57 -0800 (PST) Received: from [10.1.34.183] (e125442.arm.com [10.1.34.183]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3F6043F703; Wed, 8 Nov 2023 09:14:11 -0800 (PST) Message-ID: Date: Wed, 8 Nov 2023 17:14:09 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v6 06/23] dts: logger and settings docstring update Content-Language: en-US To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, bruce.richardson@intel.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com Cc: dev@dpdk.org References: <20231106171601.160749-1-juraj.linkes@pantheon.tech> <20231108125324.191005-1-juraj.linkes@pantheon.tech> <20231108125324.191005-6-juraj.linkes@pantheon.tech> From: Yoan Picchi In-Reply-To: <20231108125324.191005-6-juraj.linkes@pantheon.tech> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 11/8/23 12:53, Juraj Linkeš wrote: > Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linkeš > --- > dts/framework/logger.py | 72 +++++++++++++++++++++---------- > dts/framework/utils.py | 96 ++++++++++++++++++++++++++++++----------- > 2 files changed, 121 insertions(+), 47 deletions(-) > > diff --git a/dts/framework/logger.py b/dts/framework/logger.py > index bb2991e994..d3eb75a4e4 100644 > --- a/dts/framework/logger.py > +++ b/dts/framework/logger.py > @@ -3,9 +3,9 @@ > # Copyright(c) 2022-2023 PANTHEON.tech s.r.o. > # Copyright(c) 2022-2023 University of New Hampshire > > -""" > -DTS logger module with several log level. DTS framework and TestSuite logs > -are saved in different log files. > +"""DTS logger module. > + > +DTS framework and TestSuite logs are saved in different log files. > """ > > import logging > @@ -18,19 +18,21 @@ > stream_fmt = "%(asctime)s - %(name)s - %(levelname)s - %(message)s" > > > -class LoggerDictType(TypedDict): > - logger: "DTSLOG" > - name: str > - node: str > - > +class DTSLOG(logging.LoggerAdapter): > + """DTS logger adapter class for framework and testsuites. > > -# List for saving all using loggers > -Loggers: list[LoggerDictType] = [] > + The :option:`--verbose` command line argument and the :envvar:`DTS_VERBOSE` environment > + variable control the verbosity of output. If enabled, all messages will be emitted to the > + console. > > + The :option:`--output` command line argument and the :envvar:`DTS_OUTPUT_DIR` environment > + variable modify the directory where the logs will be stored. > > -class DTSLOG(logging.LoggerAdapter): > - """ > - DTS log class for framework and testsuite. > + Attributes: > + node: The additional identifier. Currently unused. > + sh: The handler which emits logs to console. > + fh: The handler which emits logs to a file. > + verbose_fh: Just as fh, but logs with a different, more verbose, format. > """ > > _logger: logging.Logger > @@ -40,6 +42,15 @@ class DTSLOG(logging.LoggerAdapter): > verbose_fh: logging.FileHandler > > def __init__(self, logger: logging.Logger, node: str = "suite"): > + """Extend the constructor with additional handlers. > + > + One handler logs to the console, the other one to a file, with either a regular or verbose > + format. > + > + Args: > + logger: The logger from which to create the logger adapter. > + node: An additional identifier. Currently unused. > + """ > self._logger = logger > # 1 means log everything, this will be used by file handlers if their level > # is not set > @@ -92,26 +103,43 @@ def __init__(self, logger: logging.Logger, node: str = "suite"): > super(DTSLOG, self).__init__(self._logger, dict(node=self.node)) > > def logger_exit(self) -> None: > - """ > - Remove stream handler and logfile handler. > - """ > + """Remove the stream handler and the logfile handler.""" > for handler in (self.sh, self.fh, self.verbose_fh): > handler.flush() > self._logger.removeHandler(handler) > > > +class _LoggerDictType(TypedDict): > + logger: DTSLOG > + name: str > + node: str > + > + > +# List for saving all loggers in use > +_Loggers: list[_LoggerDictType] = [] > + > + > def getLogger(name: str, node: str = "suite") -> DTSLOG: > + """Get DTS logger adapter identified by name and node. > + > + An existing logger will be return if one with the exact name and node already exists. > + A new one will be created and stored otherwise. > + > + Args: > + name: The name of the logger. > + node: An additional identifier for the logger. > + > + Returns: > + A logger uniquely identified by both name and node. > """ > - Get logger handler and if there's no handler for specified Node will create one. > - """ > - global Loggers > + global _Loggers > # return saved logger > - logger: LoggerDictType > - for logger in Loggers: > + logger: _LoggerDictType > + for logger in _Loggers: > if logger["name"] == name and logger["node"] == node: > return logger["logger"] > > # return new logger > dts_logger: DTSLOG = DTSLOG(logging.getLogger(name), node) > - Loggers.append({"logger": dts_logger, "name": name, "node": node}) > + _Loggers.append({"logger": dts_logger, "name": name, "node": node}) > return dts_logger > diff --git a/dts/framework/utils.py b/dts/framework/utils.py > index f0c916471c..0613adf7ad 100644 > --- a/dts/framework/utils.py > +++ b/dts/framework/utils.py > @@ -3,6 +3,16 @@ > # Copyright(c) 2022-2023 PANTHEON.tech s.r.o. > # Copyright(c) 2022-2023 University of New Hampshire > > +"""Various utility classes and functions. > + > +These are used in multiple modules across the framework. They're here because > +they provide some non-specific functionality, greatly simplify imports or just don't > +fit elsewhere. > + > +Attributes: > + REGEX_FOR_PCI_ADDRESS: The regex representing a PCI address, e.g. ``0000:00:08.0``. > +""" > + > import atexit > import json > import os > @@ -19,12 +29,20 @@ > > > def expand_range(range_str: str) -> list[int]: > - """ > - Process range string into a list of integers. There are two possible formats: > - n - a single integer > - n-m - a range of integers > + """Process `range_str` into a list of integers. > + > + There are two possible formats of `range_str`: > + > + * ``n`` - a single integer, > + * ``n-m`` - a range of integers. > > - The returned range includes both n and m. Empty string returns an empty list. > + The returned range includes both ``n`` and ``m``. Empty string returns an empty list. > + > + Args: > + range_str: The range to expand. > + > + Returns: > + All the numbers from the range. > """ > expanded_range: list[int] = [] > if range_str: > @@ -39,6 +57,14 @@ def expand_range(range_str: str) -> list[int]: > > > def get_packet_summaries(packets: list[Packet]) -> str: > + """Format a string summary from `packets`. > + > + Args: > + packets: The packets to format. > + > + Returns: > + The summary of `packets`. > + """ > if len(packets) == 1: > packet_summaries = packets[0].summary() > else: > @@ -49,6 +75,8 @@ def get_packet_summaries(packets: list[Packet]) -> str: > > > class StrEnum(Enum): > + """Enum with members stored as strings.""" > + > @staticmethod > def _generate_next_value_( > name: str, start: int, count: int, last_values: object > @@ -56,22 +84,29 @@ def _generate_next_value_( > return name > > def __str__(self) -> str: > + """The string representation is the name of the member.""" > return self.name > > > class MesonArgs(object): > - """ > - Aggregate the arguments needed to build DPDK: > - default_library: Default library type, Meson allows "shared", "static" and "both". > - Defaults to None, in which case the argument won't be used. > - Keyword arguments: The arguments found in meson_options.txt in root DPDK directory. > - Do not use -D with them, for example: > - meson_args = MesonArgs(enable_kmods=True). > - """ > + """Aggregate the arguments needed to build DPDK.""" > > _default_library: str > > def __init__(self, default_library: str | None = None, **dpdk_args: str | bool): > + """Initialize the meson arguments. > + > + Args: > + default_library: The default library type, Meson supports ``shared``, ``static`` and > + ``both``. Defaults to :data:`None`, in which case the argument won't be used. > + dpdk_args: The arguments found in ``meson_options.txt`` in root DPDK directory. > + Do not use ``-D`` with them. > + > + Example: > + :: > + > + meson_args = MesonArgs(enable_kmods=True). > + """ > self._default_library = ( > f"--default-library={default_library}" if default_library else "" > ) > @@ -83,6 +118,7 @@ def __init__(self, default_library: str | None = None, **dpdk_args: str | bool): > ) > > def __str__(self) -> str: > + """The actual args.""" > return " ".join(f"{self._default_library} {self._dpdk_args}".split()) > > > @@ -93,35 +129,33 @@ class _TarCompressionFormat(StrEnum): > and Enum values are the associated file extensions. > """ > > + #: > gzip = "gz" > + #: > compress = "Z" > + #: > bzip2 = "bz2" > + #: > lzip = "lz" > + #: > lzma = "lzma" > + #: > lzop = "lzo" > + #: > xz = "xz" > + #: > zstd = "zst" Just to be sure, _TarCompressionFormat doesn't appear in the doc (framework.utils.html). I believe that's intended (because of the _) but then I don't think the #: are used for anything. > > > class DPDKGitTarball(object): > - """Create a compressed tarball of DPDK from the repository. > - > - The DPDK version is specified with git object git_ref. > - The tarball will be compressed with _TarCompressionFormat, > - which must be supported by the DTS execution environment. > - The resulting tarball will be put into output_dir. > + """Compressed tarball of DPDK from the repository. > > - The class supports the os.PathLike protocol, > + The class supports the :class:`os.PathLike` protocol, > which is used to get the Path of the tarball:: > > from pathlib import Path > tarball = DPDKGitTarball("HEAD", "output") > tarball_path = Path(tarball) > - > - Arguments: > - git_ref: A git commit ID, tag ID or tree ID. > - output_dir: The directory where to put the resulting tarball. > - tar_compression_format: The compression format to use. > """ > > _git_ref: str > @@ -136,6 +170,17 @@ def __init__( > output_dir: str, > tar_compression_format: _TarCompressionFormat = _TarCompressionFormat.xz, > ): > + """Create the tarball during initialization. > + > + The DPDK version is specified with `git_ref`. The tarball will be compressed with > + `tar_compression_format`, which must be supported by the DTS execution environment. > + The resulting tarball will be put into `output_dir`. > + > + Args: > + git_ref: A git commit ID, tag ID or tree ID. > + output_dir: The directory where to put the resulting tarball. > + tar_compression_format: The compression format to use. > + """ > self._git_ref = git_ref > self._tar_compression_format = tar_compression_format > > @@ -204,4 +249,5 @@ def _delete_tarball(self) -> None: > os.remove(self._tarball_path) > > def __fspath__(self) -> str: > + """The os.PathLike protocol implementation.""" > return str(self._tarball_path)