chore: address PR comments

Author: nherment

holmes/plugins/toolsets/prometheus/prometheus.py

@@ -22,6 +22,7 @@
     ToolsetTag,
 )
 from holmes.plugins.toolsets.consts import STANDARD_END_DATETIME_TOOL_PARAM_DESCRIPTION
+from holmes.plugins.toolsets.prometheus.utils import parse_duration_to_seconds
 from holmes.plugins.toolsets.service_discovery import PrometheusDiscovery
 from holmes.plugins.toolsets.utils import (
     get_param_or_raise,
@@ -931,7 +932,7 @@ def __init__(self, toolset: "PrometheusToolset"):
                 "step": ToolParameter(
                     description="Query resolution step width in duration format or float number of seconds",
                     type="number",
-                    required=True,
+                    required=False,
                 ),
                 "output_type": ToolParameter(
                     description="Specifies how to interpret the Prometheus result. Use 'Plain' for raw values, 'Bytes' to format byte values, 'Percentage' to scale 0–1 values into 0–100%, or 'CPUUsage' to convert values to cores (e.g., 500 becomes 500m, 2000 becomes 2).",
@@ -961,13 +962,13 @@ def _invoke(
                 end_timestamp=params.get("end"),
                 default_time_span_seconds=DEFAULT_GRAPH_TIME_SPAN_SECONDS,
             )
-            step = params.get("step")
+            step = parse_duration_to_seconds(params.get("step"))
 
             # adjust_step_for_max_points handles None case and converts to float
             step = adjust_step_for_max_points(
                 start_timestamp=start,
                 end_timestamp=end,
-                step=float(step) if step else None,
+                step=step,
             )
 
             description = params.get("description", "")

holmes/plugins/toolsets/prometheus/utils.py

@@ -0,0 +1,30 @@
+import re
+from typing import Optional
+
+from git import Union
+
+
+def parse_duration_to_seconds(v: Optional[Union[str, float, int]]) -> Optional[float]:
+    if v is None:
+        return None
+    if isinstance(v, (int, float)):
+        return float(v)
+    s = v.strip().lower()
+    if s.isdigit():
+        return float(int(s))
+
+    units = {"s": 1, "m": 60, "h": 3600, "d": 86400}
+
+    # Check for partial time formats (e.g., 1h30m, 5m12s, 1d2h30m)
+    pattern = r"(\d+(?:\.\d+)?)(d|h|m|s)"
+    matches = re.findall(pattern, s)
+
+    if matches:
+        total_seconds = 0.0
+        for value_str, unit in matches:
+            value = float(value_str)
+            total_seconds += value * units[unit]
+        return float(int(total_seconds))
+
+    # fallback: try float seconds
+    return float(s)

tests/plugins/toolsets/prometheus/test_utils.py

@@ -0,0 +1,77 @@
+import pytest
+from holmes.plugins.toolsets.prometheus.utils import parse_duration_to_seconds
+
+
+class TestParseDurationToSeconds:
+    @pytest.mark.parametrize(
+        "input_value,expected",
+        [
+            # None case
+            (None, None),
+            # Numeric inputs (int and float)
+            (42, 42.0),
+            (3.14, 3.14),
+            (0, 0.0),
+            # String numeric inputs
+            ("123", 123.0),
+            ("0", 0.0),
+            ("  456  ", 456.0),  # with whitespace
+            # Time unit strings
+            ("30s", 30.0),
+            ("5m", 300.0),
+            ("2h", 7200.0),
+            ("1d", 86400.0),
+            # Decimal values with units
+            ("2.5s", 2.0),  # int(float(2.5) * 1) = 2
+            ("1.5m", 90.0),  # int(float(1.5) * 60) = 90
+            ("0.5h", 1800.0),  # int(float(0.5) * 3600) = 1800
+            ("0.25d", 21600.0),  # int(float(0.25) * 86400) = 21600
+            # Case insensitive and whitespace handling
+            ("30S", 30.0),
+            ("5M", 300.0),
+            ("2H", 7200.0),
+            ("1D", 86400.0),
+            ("  30s  ", 30.0),
+            # Fallback to float seconds
+            ("123.45", 123.45),
+            ("0.5", 0.5),
+            # Edge cases
+            ("10", 10.0),  # pure digit string
+            ("0s", 0.0),
+            ("0m", 0.0),
+            ("0h", 0.0),
+            ("0d", 0.0),
+            # Partial time formats
+            ("1h30m", 5400.0),  # 1*3600 + 30*60 = 5400
+            ("2h45m", 9900.0),  # 2*3600 + 45*60 = 9900
+            ("5m12s", 312.0),  # 5*60 + 12 = 312
+            ("1m30s", 90.0),  # 1*60 + 30 = 90
+            ("3h15m45s", 11745.0),  # 3*3600 + 15*60 + 45 = 11745
+            ("1d2h30m", 95400.0),  # 1*86400 + 2*3600 + 30*60 = 95400
+            ("2d1h", 176400.0),  # 2*86400 + 1*3600 = 176400
+            ("30m15s", 1815.0),  # 30*60 + 15 = 1815
+            ("1h0m30s", 3630.0),  # 1*3600 + 0*60 + 30 = 3630
+            ("0h5m", 300.0),  # 0*3600 + 5*60 = 300
+            # Case insensitive partial times
+            ("1H30M", 5400.0),
+            ("5M12S", 312.0),
+            ("  1h30m  ", 5400.0),  # with whitespace
+        ],
+    )
+    def test_parse_duration_to_seconds(self, input_value, expected):
+        result = parse_duration_to_seconds(input_value)
+        assert result == expected
+
+    @pytest.mark.parametrize(
+        "invalid_input",
+        [
+            "invalid",
+            "10x",  # unsupported unit
+            "abc123",
+            "",
+        ],
+    )
+    def test_parse_duration_to_seconds_invalid_fallback(self, invalid_input):
+        # These should raise ValueError when trying to convert to float
+        with pytest.raises(ValueError):
+            parse_duration_to_seconds(invalid_input)