DM-52898: Compile code summary only when necessary

doc/changes/DM-52898.other.rst

@@ -0,0 +1 @@
+Modified the BPS report driver so it compiles exit code summary only when necessary, i.e., when ``--return-exit-codes`` option was used with ``bps report``.

python/lsst/ctrl/bps/drivers.py

@@ -55,6 +55,7 @@
 from lsst.utils.usage import get_peak_mem_usage
 
 from . import BPS_DEFAULTS, BPS_SEARCH_ORDER, DEFAULT_MEM_FMT, DEFAULT_MEM_UNIT, BpsConfig
+from .bps_reports import compile_code_summary, compile_job_summary
 from .bps_utils import _dump_env_info, _dump_pkg_info, _make_id_link
 from .cancel import cancel
 from .construct import construct
@@ -68,7 +69,7 @@
 from .ping import ping
 from .pre_transform import acquire_quantum_graph, cluster_quanta
 from .prepare import prepare
-from .report import BPS_POSTPROCESSORS, display_report, retrieve_report
+from .report import display_report, retrieve_report
 from .restart import restart
 from .status import status
 from .submit import submit
@@ -400,20 +401,31 @@ def restart_driver(wms_service, run_id):
             print("Restart failed: Unknown error")
 
 
-def report_driver(wms_service, run_id, user, hist_days, pass_thru, is_global=False, return_exit_codes=False):
-    """Print out summary of jobs submitted for execution.
+def report_driver(
+    wms_service: str | None = None,
+    run_id: str | None = None,
+    user: str | None = None,
+    hist_days: float = 0.0,
+    pass_thru: str | None = None,
+    is_global: bool = False,
+    return_exit_codes: bool = False,
+):
+    """Print out the summary of jobs submitted for execution.
 
     Parameters
     ----------
-    wms_service : `str`
+    wms_service : `str`, optional
         Name of the class.
-    run_id : `str`
+    run_id : `str`, optional
         A run id the report will be restricted to.
-    user : `str`
+    user : `str`, optional
         A user the report will be restricted to.
-    hist_days : `float`
-        Number of days.
-    pass_thru : `str`
+    hist_days : `float`, optional
+        Number of past days to consider while preparing the report. By default,
+        only the currently running workflows are included in the report.
+        If the report is restricted to a single run (i.e., ``run_id`` is set),
+        the history search will be limited by default to two past days.
+    pass_thru : `str`, optional
         A string to pass directly to the WMS service class.
     is_global : `bool`, optional
         If set, all available job queues will be queried for job information.
@@ -430,17 +442,19 @@ def report_driver(wms_service, run_id, user, hist_days, pass_thru, is_global=Fal
         Only applicable in the context of a WMS with associated
         handlers to return exit codes from jobs.
     """
-    if wms_service is None:
+    if not wms_service:
         default_config = BpsConfig(BPS_DEFAULTS)
         wms_service = os.environ.get("BPS_WMS_SERVICE_CLASS", default_config["wmsServiceClass"])
 
-    # When reporting on single run:
-    # * increase history until better mechanism for handling completed jobs is
-    #   available.
+    # When reporting on a single run:
+    # * increase history until a better mechanism for handling completed jobs
+    #   is available.
     # * massage the retrieved reports using BPS report postprocessors.
     if run_id:
         hist_days = max(hist_days, 2)
-        postprocessors = BPS_POSTPROCESSORS
+        postprocessors = [compile_job_summary]
+        if return_exit_codes:
+            postprocessors.append(compile_code_summary)
     else:
         postprocessors = None
 

python/lsst/ctrl/bps/report.py

@@ -31,7 +31,7 @@
 oriented database tables.
 """
 
-__all__ = ["BPS_POSTPROCESSORS", "display_report", "retrieve_report"]
+__all__ = ["display_report", "retrieve_report"]
 
 import logging
 import sys
@@ -40,20 +40,9 @@
 
 from lsst.utils import doImportType
 
-from .bps_reports import (
-    DetailedRunReport,
-    ExitCodesReport,
-    SummaryRunReport,
-    compile_code_summary,
-    compile_job_summary,
-)
+from .bps_reports import DetailedRunReport, ExitCodesReport, SummaryRunReport
 from .wms_service import BaseWmsService, WmsRunReport, WmsStates
 
-BPS_POSTPROCESSORS = (compile_job_summary, compile_code_summary)
-"""Postprocessors for massaging run reports
-(`tuple` [`Callable` [[`WmsRunReport`], None]).
-"""
-
 _LOG = logging.getLogger(__name__)
 
 
@@ -167,7 +156,7 @@ def retrieve_report(
     pass_thru: str | None = None,
     is_global: bool = False,
     return_exit_codes: bool = False,
-    postprocessors: Sequence[Callable[[WmsRunReport], None]] | None = None,
+    postprocessors: Sequence[Callable[[WmsRunReport], list[str]]] | None = None,
 ) -> tuple[list[WmsRunReport], list[str]]:
     """Retrieve summary of jobs submitted for execution.
 

tests/test_drivers.py

@@ -34,8 +34,9 @@
 
 import yaml
 
-from lsst.ctrl.bps import WmsStates
-from lsst.ctrl.bps.drivers import _init_submission_driver, ping_driver, status_driver
+from lsst.ctrl.bps import WmsRunReport, WmsStates
+from lsst.ctrl.bps.bps_reports import compile_code_summary, compile_job_summary
+from lsst.ctrl.bps.drivers import _init_submission_driver, ping_driver, report_driver, status_driver
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
 
@@ -142,5 +143,187 @@ def testWmsServiceNone(self):
             self.assertEqual(retval, WmsStates.RUNNING.value)
 
 
+class TestReportDriver(unittest.TestCase):
+    """Test report_driver function."""
+
+    @unittest.mock.patch(
+        "lsst.ctrl.bps.drivers.BPS_DEFAULTS", new={"wmsServiceClass": "wms_test_utils.WmsServiceSuccess"}
+    )
+    def testWmsServiceFromDefaults(self):
+        # Should not raise an exception and use default from BPS_DEFAULTS.
+        with unittest.mock.patch.dict(os.environ, {}, clear=True):
+            report_driver(
+                wms_service=None,
+                run_id=None,
+                user=None,
+                hist_days=0,
+                pass_thru=None,
+            )
+
+    def testWmsServiceFromEnvVar(self):
+        # Should not raise an exception.
+        with unittest.mock.patch.dict(
+            os.environ, {"BPS_WMS_SERVICE_CLASS": "wms_test_utils.WmsServiceSuccess"}
+        ):
+            report_driver(
+                wms_service=None,
+                run_id=None,
+                user=None,
+                hist_days=0.0,
+                pass_thru=None,
+            )
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testHistDefault(self, mock_display, mock_retrieve):
+        mock_retrieve.return_value = ([], [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id="123",
+            user=None,
+            hist_days=0.0,
+            pass_thru=None,
+        )
+
+        # Verify retrieve_report was called with the default hist setting.
+        _, kwargs = mock_retrieve.call_args
+        self.assertAlmostEqual(kwargs["hist"], 2.0)
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testHistCustom(self, mock_display, mock_retrieve):
+        mock_retrieve.return_value = ([], [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id="123",
+            user=None,
+            hist_days=4.0,
+            pass_thru=None,
+        )
+
+        # Verify retrieve_report was called with a custom hist setting.
+        _, kwargs = mock_retrieve.call_args
+        self.assertAlmostEqual(kwargs["hist"], 4.0)
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testPostprocessorsWithoutExitCodes(self, mock_display, mock_retrieve):
+        mock_retrieve.return_value = ([], [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id="123",
+            user=None,
+            hist_days=0.0,
+            pass_thru=None,
+            return_exit_codes=False,
+        )
+
+        # Verify the postprocessors list contains only one postprocessor.
+        args, kwargs = mock_retrieve.call_args
+        self.assertEqual(len(kwargs["postprocessors"]), 1)
+        self.assertIn(compile_job_summary, kwargs["postprocessors"])
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testPostprocessorsWithExitCodes(self, mock_display, mock_retrieve):
+        mock_retrieve.return_value = ([], [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id="123",
+            user=None,
+            hist_days=0.0,
+            pass_thru=None,
+            return_exit_codes=True,
+        )
+
+        # Verify the postprocessors list contains both postprocessors.
+        _, kwargs = mock_retrieve.call_args
+        self.assertEqual(len(kwargs["postprocessors"]), 2)
+        self.assertIn(compile_code_summary, kwargs["postprocessors"])
+        self.assertIn(compile_job_summary, kwargs["postprocessors"])
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testPostprocessorsNoRunId(self, mock_display, mock_retrieve):
+        mock_retrieve.return_value = ([], [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id=None,
+            user=None,
+            hist_days=0.0,
+            pass_thru=None,
+        )
+
+        # Verify postprocessors contains compile_job_summary
+        _, kwargs = mock_retrieve.call_args
+        self.assertIsNone(kwargs["postprocessors"])
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testDisplayCalledIfRuns(self, mock_display, mock_retrieve):
+        mock_runs = [WmsRunReport(wms_id="1", state=WmsStates.SUCCEEDED)]
+        mock_retrieve.return_value = (mock_runs, [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id=None,
+            user=None,
+            hist_days=0,
+            pass_thru=None,
+        )
+
+        # Verify display_report was called with the runs
+        mock_display.assert_called_once()
+        args, kwargs = mock_display.call_args
+        self.assertEqual(args[0], mock_runs)
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    def testDisplayCalledIfMessages(self, mock_display, mock_retrieve):
+        mock_messages = ["Warning message 1", "Warning message 2"]
+        mock_retrieve.return_value = ([], mock_messages)
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id=None,
+            user=None,
+            hist_days=0,
+            pass_thru=None,
+        )
+
+        # Verify display_report was called with messages
+        mock_display.assert_called_once()
+        args, kwargs = mock_display.call_args
+        self.assertEqual(args[1], mock_messages)
+
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.retrieve_report")
+    @unittest.mock.patch("lsst.ctrl.bps.drivers.display_report")
+    @unittest.mock.patch("builtins.print")
+    def testNoRecordsFoundMessage(self, mock_print, mock_display, mock_retrieve):
+        mock_retrieve.return_value = ([], [])
+
+        report_driver(
+            wms_service="wms_test_utils.WmsServiceSuccess",
+            run_id="123",
+            user=None,
+            hist_days=1.5,
+            pass_thru=None,
+        )
+
+        # Verify display_report() was NOT called.
+        mock_display.assert_not_called()
+
+        # Verify that a helpful message was printed.
+        mock_print.assert_called_once()
+        call_args = mock_print.call_args[0][0]
+        self.assertIn("No records found", call_args)
+        self.assertIn("123", call_args)
+
+
 if __name__ == "__main__":
     unittest.main()