Repository: maxfischer2781/cms_perf

Issue:
Allow CLI from file
Support from-file arguments in the CLI, to support basic configuration files.

Discussion:

Patch:
diff --git a/.coveragerc b/.coveragerc
index 588b627..969c225 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -16,3 +16,4 @@ exclude_lines =
     if __name__ == '__main__'
     @overload
     if TYPE_CHECKING
+    ...
diff --git a/cms_perf/report.py b/cms_perf/report.py
index e0c5442..b1a21a4 100644
--- a/cms_perf/report.py
+++ b/cms_perf/report.py
@@ -5,8 +5,8 @@ from typing import Callable
 import sys
 import time
 
-from .cli import CLI
-from . import cli_parser
+from .setup.cli import CLI
+from .setup import cli_parser
 
 
 class PseudoSched:
@@ -21,7 +21,7 @@ class PseudoSched:
         self.maxload = maxload
 
     @classmethod
-    def from_directive(cls, directive: str):
+    def from_directive(cls, directive: str) -> "PseudoSched":
         """Create an instance by parsing a ``cms.sched`` directive"""
         items = directive.split()
         policy = {
@@ -76,16 +76,10 @@ def run_forever(
     sched: PseudoSched = None,
 ):
     """Write sensor information to stdout every ``interval`` seconds"""
-    sensors = (
-        prunq,
-        pcpu,
-        pmem,
-        ppag,
-        pio,
-    )
+    sensors = (prunq, pcpu, pmem, ppag, pio)
     try:
         for _ in every(interval):
-            (*values,) = map(clamp_percentages, map(lambda x: x(), sensors,),)
+            values = [clamp_percentages(sensor()) for sensor in sensors]
             print(*values, end="", flush=True)
             if sched is not None:
                 load, rejected = sched.weight(*values)
diff --git a/cms_perf/sensors/__init__.py b/cms_perf/sensors/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/cms_perf/net_load.py b/cms_perf/sensors/net_load.py
similarity index 93%
rename from cms_perf/net_load.py
rename to cms_perf/sensors/net_load.py
index c6f8791..cb3c0f4 100644
--- a/cms_perf/net_load.py
+++ b/cms_perf/sensors/net_load.py
@@ -5,7 +5,7 @@ import enum
 
 import psutil
 
-from .cli_parser import cli_domain, cli_call
+from ..setup.cli_parser import cli_domain, cli_call
 
 
 @cli_domain(name="KIND")
diff --git a/cms_perf/sensor.py b/cms_perf/sensors/sensor.py
similarity index 97%
rename from cms_perf/sensor.py
rename to cms_perf/sensors/sensor.py
index f7b1988..581eeef 100644
--- a/cms_perf/sensor.py
+++ b/cms_perf/sensors/sensor.py
@@ -11,7 +11,7 @@ import enum
 
 import psutil
 
-from .cli_parser import cli_call, cli_domain
+from ..setup.cli_parser import cli_call, cli_domain
 
 
 # individual sensors for system state
diff --git a/cms_perf/xrd_load.py b/cms_perf/sensors/xrd_load.py
similarity index 96%
rename from cms_perf/xrd_load.py
rename to cms_perf/sensors/xrd_load.py
index d1f95e6..cea5af6 100644
--- a/cms_perf/xrd_load.py
+++ b/cms_perf/sensors/xrd_load.py
@@ -1,12 +1,12 @@
 """
-P
+Sensors for resources used by XRootD processes
 """
 from typing import List
 import time
 
 import psutil
 
-from .cli_parser import cli_call
+from ..setup.cli_parser import cli_call
 
 
 def rescan(interval):
diff --git a/cms_perf/setup/__init__.py b/cms_perf/setup/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/cms_perf/cli.py b/cms_perf/setup/cli.py
similarity index 59%
rename from cms_perf/cli.py
rename to cms_perf/setup/cli.py
index e7d447e..c85690b 100644
--- a/cms_perf/cli.py
+++ b/cms_perf/setup/cli.py
@@ -1,12 +1,29 @@
 import argparse
 
-from . import cli_parser
-from . import __version__ as lib_version
+from ..setup import cli_parser
+from .. import __version__ as lib_version
 
 # ensure sensors are loaded
-from . import sensor  # noqa
-from . import xrd_load  # noqa
-from . import net_load  # noqa
+from ..sensors import sensor, xrd_load, net_load  # noqa
+
+
+class ConfigArgumentParser(argparse.ArgumentParser):
+    """
+    Custom parser for ini-style config files, provided as ``@/path/to/config``
+
+    Allows comments and the use of key value notation, as in::
+
+        # Set the --prunq option
+        prunq = 100.0*loadq/40/ncores
+    """
+
+    def convert_arg_line_to_args(self, arg_line):
+        arg_line, *_ = arg_line.split("#", 1)
+        if not arg_line.strip():
+            return []
+        key, _, value = [elem.strip() for elem in arg_line.partition("=")]
+        key = key if key.startswith("--") else ("--" + key)
+        return [key, value] if value else [key]
 
 
 INTERVAL_UNITS = {"": 1, "s": 1, "m": 60, "h": 60 * 60}
@@ -34,7 +51,7 @@ def duration(literal: str) -> float:
     return float(value) * scale
 
 
-CLI = argparse.ArgumentParser(
+CLI = ConfigArgumentParser(
     description="Performance Sensor for XRootD cms.perf directive",
     epilog=(
         "In regular intervals, outputs a single line with percentages of: "
@@ -46,45 +63,46 @@ CLI = argparse.ArgumentParser(
         "The paging load exists for historical reasons; "
         "it cannot be reliably computed."
     ),
+    fromfile_prefix_chars="@",
 )
 CLI.add_argument(
     "--version", action="version", version=lib_version,
 )
 CLI.add_argument(
     "--interval",
-    default=60,
-    help="Interval between output; suffixed by s (default), m, or h",
+    default="60s",
+    help="Interval between output; suffixed by s, m, or h [default: %(default)s]",
     type=duration,
 )
 CLI.add_argument(
     "--prunq",
     default="100.0*loadq/ncores",
     type=cli_parser.parse_sensor,
-    help="Expression to compute system load percentage",
+    help="Expression to compute system load percentage [default: %(default)s]",
 )
 CLI.add_argument(
     "--pcpu",
     default="pcpu",
     type=cli_parser.parse_sensor,
-    help="Expression to compute cpu utilization percentage",
+    help="Expression to compute cpu utilization percentage [default: %(default)s]",
 )
 CLI.add_argument(
     "--pmem",
     default="pmem",
     type=cli_parser.parse_sensor,
-    help="Expression to compute memory utilization percentage",
+    help="Expression to compute memory utilization percentage [default: %(default)s]",
 )
 CLI.add_argument(
     "--ppag",
     default="0",
     type=cli_parser.parse_sensor,
-    help="Expression to compute paging load percentage",
+    help="Expression to compute paging load percentage [default: %(default)s]",
 )
 CLI.add_argument(
     "--pio",
     default="pio",
     type=cli_parser.parse_sensor,
-    help="Expression to compute network utilization percentage",
+    help="Expression to compute network utilization percentage [default: %(default)s]",
 )
 CLI.add_argument(
     "--sched",
diff --git a/cms_perf/cli_parser.py b/cms_perf/setup/cli_parser.py
similarity index 99%
rename from cms_perf/cli_parser.py
rename to cms_perf/setup/cli_parser.py
index 1344542..e55e3c9 100644
--- a/cms_perf/cli_parser.py
+++ b/cms_perf/setup/cli_parser.py
@@ -278,7 +278,7 @@ def minimum(a, b, *others):
 
 if __name__ == "__main__":
     # provide debug information on the parser
-    from . import sensor, net_load, xrd_load  # noqa
+    from ..sensors import sensor, net_load, xrd_load  # noqa
     from . import cli_parser  # noqa
 
     print("EXPRESSION:", cli_parser.EXPRESSION)
diff --git a/docs/generate_docs.py b/docs/generate_docs.py
index caddf1f..3109b48 100644
--- a/docs/generate_docs.py
+++ b/docs/generate_docs.py
@@ -2,9 +2,7 @@ from pathlib import Path
 import inspect
 import textwrap
 
-from cms_perf import cli
-from cms_perf import cli_parser
-
+from cms_perf.setup import cli_parser, cli
 
 TARGET_DIR = Path(__file__).parent / "generated"
 TARGET_DIR.mkdir(exist_ok=True)
@@ -17,7 +15,7 @@ def normalized_doc(obj):
 def document_cli_sensors():
     rst_lines = []
     for action in cli.CLI._actions:
-        if action.type != cli.cli_parser.parse_sensor:
+        if action.type != cli_parser.parse_sensor:
             continue
         cli_name = max(action.option_strings, key=len)
         rst_lines.append(f"``{cli_name}={action.default}``\n   {action.help}\n")
diff --git a/docs/source/setup.rst b/docs/source/setup.rst
index fa6ada5..c33cd62 100644
--- a/docs/source/setup.rst
+++ b/docs/source/setup.rst
@@ -77,6 +77,43 @@ Use the same interval for the directive's ``int`` and the sensor's ``--interval`
 
 See the `cms.perf documentation`_ for details of the directive.
 
+Sensor Configuration
+====================
+
+The ``cms_perf`` sensor can be configured for report interval,
+:doc:`load sensors <./setup>` and :doc:`cms.sched emulation <./sched>`.
+The CLI supports both directly specified options as well as
+ini-style configuration files.
+
+.. tabs::
+
+    .. group-tab:: Explicit Option
+
+        .. code::
+
+            cms_perf --interval 10
+
+    .. group-tab:: Configuration File
+
+        .. code::
+
+            cms_perf @/etc/cms_perf.ini
+
+The configuration file uses ``option = value``
+when the CLI would use ``--option value``.
+Only one option per line is allowed;
+``#`` marks comments for the rest of the line and empty lines are ignored.
+
+.. code::
+
+    interval = 60s
+    # Redefine prunq and ppag based on machine size
+    prunq = 100.0*loadq/10/ncores
+    pcpu = pcpu
+    pmem = pmem
+    ppag = 100.0*nsockets/250/ncores
+    pio = pio
+
 .. _virtual environment: https://docs.python.org/3/library/venv.html
 .. _psutil documentation: https://psutil.readthedocs.io/
 .. _cms.perf documentation: https://xrootd.slac.stanford.edu/doc/dev410/cms_config.htm#_Toc8247264


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index f6a0a88..4617524 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -3,6 +3,7 @@ import sys
 
 import pytest
 import coverage
+import tempfile
 
 from .utility import capture
 from . import mimicry
@@ -51,6 +52,32 @@ def test_run_replaced(executable: List[str]):
             assert idx == int(reading)
 
 
+@pytest.mark.parametrize("executable", EXECUTABLES)
+def test_run_fromfile(executable: List[str]):
+    with tempfile.NamedTemporaryFile() as tf:
+        tf.writelines(
+            [
+                b"interval=0.02\n",
+                b"# this is ignored\n",
+                b"prunq = 0\n",
+                b"pcpu= 1\n",
+                b"pmem =2\n",
+                b"ppag=3\n",
+                b"pio = 4\n",
+            ]
+        )
+        tf.flush()
+        with open(tf.name) as ist:
+            print(ist.read(), file=sys.stderr)
+        output = capture([*executable, f"@{tf.name}"], num_lines=5)
+    assert output
+    for line in output:
+        readings = line.split()
+        assert len(readings) == 5
+        for idx, reading in enumerate(readings):
+            assert idx == int(reading)
+
+
 SCHED_FIELD = tuple(enumerate(("runq", "cpu", "mem", "pag", "io")))
 
 
diff --git a/tests/test_cli_parser.py b/tests/test_cli_parser.py
index 0cd8a42..a98af45 100644
--- a/tests/test_cli_parser.py
+++ b/tests/test_cli_parser.py
@@ -4,8 +4,8 @@ import platform
 
 import psutil
 
-from cms_perf import cli_parser
-from cms_perf import (  # noqa
+from cms_perf.setup import cli_parser
+from cms_perf.sensors import (  # noqa
     sensor as _mount_sensors,
     net_load as _mount_net_load,
     xrd_load as _mount_xrd_load,
diff --git a/tests/test_sensors.py b/tests/test_sensors.py
index 4d9aa38..85fa8bc 100644
--- a/tests/test_sensors.py
+++ b/tests/test_sensors.py
@@ -1,6 +1,6 @@
 import pytest
 
-from cms_perf import sensor
+from cms_perf.sensors import sensor
 
 SENSORS = (
     sensor.system_prunq,
diff --git a/tests/test_xrd_load.py b/tests/test_xrd_load.py
index 9dae2dd..80670fa 100644
--- a/tests/test_xrd_load.py
+++ b/tests/test_xrd_load.py
@@ -1,7 +1,7 @@
 import pytest
 import psutil
 
-from cms_perf import xrd_load
+from cms_perf.sensors import xrd_load
 
 from . import mimicry