Merge pull request #6638 from grondo/issue#6625

fix incorrect combination of lines in `flux resource status` for expandable fields that differ past the minimum width

src/bindings/python/flux/util.py

@@ -618,7 +618,9 @@ def get_field(self, field_name, args, kwargs):
                 return kwargs[field_name], None
             return super().get_field(field_name, args, kwargs)
 
-    def __init__(self, fmt, headings=None, prepend="0."):
+    def __init__(
+        self, fmt, headings=None, prepend="0.", nullify_expansion_sentinel=False
+    ):
         """
         Parse the input format ``fmt`` with string.Formatter.
         Save off the fields and list of format tokens for later use,
@@ -628,6 +630,9 @@ def __init__(self, fmt, headings=None, prepend="0."):
             headings (dict): Set a mapping of field name to header string.
             prepend (str): Prepend each field with a string. (Default is
                 "0.", so ``{f1} {f2}`` becomes ``{0.f1} {0.f2}``.
+            nullify_expansion_sentinel (bool): Replace fields with an expansion
+                sentinel (``+:``) with just the field, e.g. ``+:{foo:<10.10}``
+                is replaced with ``{foo}``.
         """
         if headings is not None:
             self.headings = headings
@@ -813,7 +818,11 @@ def header(self):
         return formatter.format(self.header_format(), **self.headings)
 
     def get_format_prepended(
-        self, prepend, except_fields=None, include_sort_prefix=True
+        self,
+        prepend,
+        except_fields=None,
+        include_sort_prefix=True,
+        nullify_expansion_sentinel=False,
     ):
         """
         Return the format string, ensuring that the string in "prepend"
@@ -825,6 +834,9 @@ def get_format_prepended(
                 the format.
             include_sort_prefix (bool): If format specifies a list of sort
                 keys, include them in a ``sort:`` prefix. Default: True
+            nullify_expansion_sentinel (bool): If format specifies an expansion
+                prefix ``+:``, then substitute the field without any width
+                and precision, e.g. ``+:{foo:<10.10}`` becomes ``{foo}``.
         """
         if except_fields is None:
             except_fields = []
@@ -833,11 +845,19 @@ def get_format_prepended(
             # Skip this field if it is in except_fields
             if field in except_fields:
                 # Preserve any format "prefix" (i.e. the text):
-                lst.append(text)
+                # Unless it is whitespace or a sentinel (+:,  etc):
+                result = text.strip()
+                if result and result not in ("+:", "?:", "?+:"):
+                    lst.append(text)
                 continue
             # If field doesn't have 'prepend' then add it
             if field and not field.startswith(prepend):
                 field = prepend + field
+            # If caller has requested `+:` prefix is to be nullified,
+            # and that prefix exists, then reset text and spec:
+            if nullify_expansion_sentinel and text.endswith("+:"):
+                text = ""
+                spec = ""
             lst.append(self._fmt_tuple(text, field, spec, conv))
         if include_sort_prefix:
             prefix = self._sort_prefix()
@@ -864,17 +884,22 @@ def get_format(self, orig=False, include_sort_prefix=True):
             return self._sort_prefix() + fmt
         return fmt
 
-    def copy(self, except_fields=None):
+    def copy(self, except_fields=None, nullify_expansion=False):
         """
         Return a copy of the current formatter, optionally with some
         fields removed
 
         Args:
             except_fields (list): List of fields to remove from result.
+            nullify_expansion (bool): If True, replace any fields in original
+                that have an expansion sentinel with just the field, e.g.
+                ``+:{foo:>5.5}`` becomes ``{foo}`` in the copy.
         """
         cls = self.__class__
         return cls(
-            self.get_format_prepended("", except_fields),
+            self.get_format_prepended(
+                "", except_fields, nullify_expansion_sentinel=nullify_expansion
+            ),
             headings=self.headings,
             prepend=self.prepend,
         )
@@ -1110,7 +1135,9 @@ class Deduplicator:
     """
 
     def __init__(self, formatter, except_fields=None, combine=None):
-        self.formatter = formatter.copy(except_fields=except_fields)
+        self.formatter = formatter.copy(
+            except_fields=except_fields, nullify_expansion=True
+        )
         self.combine = combine
         self.hash = {}
         self.items = []

src/cmd/flux-resource.py

@@ -45,7 +45,7 @@ class FluxResourceConfig(UtilConfig):
             "description": "Long flux-resource status format string",
             "format": (
                 "{state:>12} {color_up}{up:>2}{color_off} "
-                "{nnodes:>6} +:{reason:<30.30+} {nodelist}"
+                "{nnodes:>6} +:{reason:<30.30} {nodelist}"
             ),
         },
     }
@@ -54,7 +54,7 @@ class FluxResourceConfig(UtilConfig):
             "description": "Long flux-resource drain format string",
             "format": (
                 "{timestamp!d:%b%d %R::<12} {state:<8.8} {ranks:<8.8+} "
-                "+:{reason:<30.30+} {nodelist}"
+                "+:{reason:<30.30} {nodelist}"
             ),
         },
         "default": {

t/python/t0024-util.py

@@ -261,6 +261,26 @@ def test_issue6530(self):
         ).filter(items)
         self.assertEqual(fmt, "{s:16.16} {i:4d} {f:3.2f}")
 
+    def test_copy(self):
+        original = "+:{s:5.5} {i:4d} {f:.2f}"
+
+        fmt = OutputFormat(original, headings=self.headings)
+        self.assertEqual(fmt.get_format_prepended(""), original)
+
+        # copy preserves original fmt
+        self.assertEqual(fmt.copy().get_format_prepended(""), original)
+
+        # except_fields can remove fields
+        self.assertEqual(
+            fmt.copy(except_fields=["s", "f"]).get_format_prepended(""), " {i:4d}"
+        )
+
+        # nullify_expansion removes formatting on +: fields
+        self.assertEqual(
+            fmt.copy(nullify_expansion=True).get_format_prepended(""),
+            "{s} {i:4d} {f:.2f}",
+        )
+
 
 if __name__ == "__main__":
     unittest.main(testRunner=TAPTestRunner())

t/t2354-resource-status.t

@@ -43,6 +43,17 @@ test_expect_success 'cancel running jobs' '
 	flux cancel --all &&
 	flux queue idle
 '
+# issue#6625:
+test_expect_success 'flux-resource status does not combine dissimilar drain reasons' '
+	test_when_finished "flux resource undrain -f 0-3" &&
+	flux resource drain -f 0-3 xxxxxxxx &&
+	flux resource drain -f 0   xxxxxxxxyy &&
+	flux resource status -no "{state:>12} {ranks:>6} +:{reason:<5.5+}" \
+		> drain.1 &&
+	flux resource status -no "{state:>12} {ranks:>6} {reason:<10}" \
+		> drain.2 &&
+	test_cmp drain.1 drain.2
+'
 test_expect_success 'flux-resource status shows housekeeping by default' '
 	flux config load <<-EOF &&
 	[job-manager.housekeeping]